You are viewing a plain text version of this content. The canonical link for it is here.
Posted to github@arrow.apache.org by "WillAyd (via GitHub)" <gi...@apache.org> on 2025/08/07 20:19:05 UTC
[PR] Meson win support [arrow]
WillAyd opened a new pull request, #47282:
URL: https://github.com/apache/arrow/pull/47282
### Rationale for this change
This allows users on the Windows platform to use the Meson build system
### What changes are included in this PR?
Updates to the Meson configurations and source files to accommodate Windows
### Are these changes tested?
Yes
### Are there any user-facing changes?
No
--
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: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] GH-46797: [C++] Support Meson on Windows [arrow]
Posted by "kou (via GitHub)" <gi...@apache.org>.
kou commented on code in PR #47282:
URL: https://github.com/apache/arrow/pull/47282#discussion_r2292474909
##########
cpp/src/arrow/flight/meson.build:
##########
@@ -44,7 +44,17 @@ install_headers(
subdir: 'arrow/flight',
)
-grpc_dep = dependency('grpc++')
+# We specifically need to use the grpc subproject and cannot use a system installion
+# due to https://github.com/mesonbuild/meson/issues/14905
Review Comment:
Oh... I see.
--
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: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] GH-46797: [C++] Support Meson on Windows [arrow]
Posted by "WillAyd (via GitHub)" <gi...@apache.org>.
WillAyd commented on code in PR #47282:
URL: https://github.com/apache/arrow/pull/47282#discussion_r2263016632
##########
cpp/src/arrow/meson.build:
##########
@@ -484,14 +505,22 @@ if needs_compute
arrow_compute_lib = library(
'arrow-compute',
sources: arrow_compute_lib_sources,
- dependencies: arrow_dep,
+ dependencies: [arrow_dep] + int128_deps,
Review Comment:
This is to build the library itself, not to expose it to consumers. I believe that MSVC/Windows do not offer a native 128 bit integer, so the `int128_deps` grabs it from the boost multiprecision header. On other platforms, this dependency is empty
--
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: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] GH-46797: [C++] Support Meson on Windows [arrow]
Posted by "WillAyd (via GitHub)" <gi...@apache.org>.
WillAyd commented on code in PR #47282:
URL: https://github.com/apache/arrow/pull/47282#discussion_r2333485992
##########
cpp/src/arrow/meson.build:
##########
@@ -420,19 +481,38 @@ foreach key, val : arrow_components
arrow_deps += val.get('dependencies', [])
endforeach
+arrow_lib_cpp_shared_args = ['-DARROW_EXPORTING', '-DURI_LIBRARY_BUILD']
+if needs_compute
+ arrow_lib_cpp_shared_args += ['-DARROW_COMPUTE_EXPORTING']
+endif
+
arrow_lib = library(
'arrow',
sources: arrow_srcs,
include_directories: arrow_includes,
dependencies: arrow_deps,
install: true,
gnu_symbol_visibility: 'inlineshidden',
- cpp_shared_args: ['-DARROW_EXPORTING'],
+ gnu_symbol_visibility: 'hidden',
Review Comment:
Nope - mistaken rebase. Thanks!
--
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: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] GH-46797: [C++] Support Meson on Windows [arrow]
Posted by "WillAyd (via GitHub)" <gi...@apache.org>.
WillAyd commented on PR #47282:
URL: https://github.com/apache/arrow/pull/47282#issuecomment-3285556171
Hmm I think that parquet PR that was merged is going to require fixes to the symbol visibility first before we can get Windows to work. I'll tackle that in a separate PR
--
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: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] GH-46797: [C++] Support Meson on Windows [arrow]
Posted by "WillAyd (via GitHub)" <gi...@apache.org>.
WillAyd commented on PR #47282:
URL: https://github.com/apache/arrow/pull/47282#issuecomment-3293207506
> Looks like building thrift as a subproject on Windows is failing to locate the Boost dependency.
Just to clarify this isn't specific to Windows, but any system where Boost is not available. It looks like the root cause is that Meson cannot guarantee that CMake subprojects are able to locate dependencies satisfied by other subprojects. There's some upstream discussion of this in the following links:
https://github.com/mesonbuild/meson/issues/12451
https://github.com/mesonbuild/meson/issues/8089
It seems rather non-trivial to fix, so for now we may just want to warn when parquet is enabled, thrift must be built as a subproject, and boost is not available on the system to build that?
We can alternatively see if Thrift can provide a Meson configuration for its next release
--
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: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] GH-46797: [C++] Support Meson on Windows [arrow]
Posted by "WillAyd (via GitHub)" <gi...@apache.org>.
WillAyd commented on PR #47282:
URL: https://github.com/apache/arrow/pull/47282#issuecomment-3192505621
OK looks like this is all green now. The Ubuntu Meson failure is unrelated and will be fixed by https://github.com/apache/arrow/pull/47041
--
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: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] GH-46797: [C++] Support Meson on Windows [arrow]
Posted by "WillAyd (via GitHub)" <gi...@apache.org>.
WillAyd commented on code in PR #47282:
URL: https://github.com/apache/arrow/pull/47282#discussion_r2336983269
##########
cpp/src/arrow/meson.build:
##########
@@ -420,19 +481,38 @@ foreach key, val : arrow_components
arrow_deps += val.get('dependencies', [])
endforeach
+arrow_lib_cpp_shared_args = ['-DARROW_EXPORTING', '-DURI_LIBRARY_BUILD']
+if needs_compute
+ arrow_lib_cpp_shared_args += ['-DARROW_COMPUTE_EXPORTING']
+endif
+
arrow_lib = library(
'arrow',
sources: arrow_srcs,
include_directories: arrow_includes,
dependencies: arrow_deps,
install: true,
gnu_symbol_visibility: 'inlineshidden',
- cpp_shared_args: ['-DARROW_EXPORTING'],
+ gnu_symbol_visibility: 'hidden',
+ c_shared_args: [
+ '-DARROW_EXPORTING',
+ '-DURI_LIBRARY_BUILD',
+ '-DURI_VISIBILITY',
Review Comment:
Hmm trying it again it doesn't appear to matter whether `-DURI_VISIBILITY` is set or not - let's remove for now.
> BTW, why do we need to use `-DURI_LIBRARY_BUILD`? CMake uses `URI_STATIC_BUILD` because vendored/uriparser is built into `libarrow`:
I think you are correct that on non-Windows platforms we may not require this macro, but on Windows removing this generates a ton of linker warnings:
<details>
```
[5/18] Compiling C object src/arrow/arrow.dll.p/vendored_uriparser_UriMemory.c.obj
../src/arrow/vendored/uriparser/UriMemory.c(129): warning C4273: 'uriEmulateCalloc': inconsistent dll linkage
C:\Users\Alison\clones\arrow\cpp\src\arrow\vendored\uriparser\UriBase.h(328): note: see previous definition of 'uriEmulateCalloc'
../src/arrow/vendored/uriparser/UriMemory.c(151): warning C4273: 'uriEmulateReallocarray': inconsistent dll linkage
C:\Users\Alison\clones\arrow\cpp\src\arrow\vendored\uriparser\UriBase.h(348): note: see previous definition of 'uriEmulateReallocarray'
../src/arrow/vendored/uriparser/UriMemory.c(264): warning C4273: 'uriCompleteMemoryManager': inconsistent dll linkage
C:\Users\Alison\clones\arrow\cpp\src\arrow\vendored\uriparser\UriBase.h(309): note: see previous definition of 'uriCompleteMemoryManager'
../src/arrow/vendored/uriparser/UriMemory.c(288): warning C4273: 'uriTestMemoryManager': inconsistent dll linkage
C:\Users\Alison\clones\arrow\cpp\src\arrow\vendored\uriparser\UriBase.h(373): note: see previous definition of 'uriTestMemoryManager'
[6/18] Compiling C object src/arrow/arrow.dll.p/vendored_uriparser_UriIp4.c.obj
C:\Users\Alison\clones\arrow\cpp\src\arrow\vendored\uriparser\UriIp4.c(94): warning C4273: 'uriParseIpFourAddressA': inconsistent dll linkage
C:\Users\Alison\clones\arrow\cpp\src\arrow\vendored\uriparser\UriIp4.h(98): note: see previous definition of 'uriParseIpFourAddressA'
C:\Users\Alison\clones\arrow\cpp\src\arrow\vendored\uriparser\UriIp4.c(94): warning C4273: 'uriParseIpFourAddressW': inconsistent dll linkage
C:\Users\Alison\clones\arrow\cpp\src\arrow\vendored\uriparser\UriIp4.h(98): note: see previous definition of 'uriParseIpFourAddressW'
[7/18] Compiling C object src/arrow/arrow.dll.p/vendored_uriparser_UriCompare.c.obj
C:\Users\Alison\clones\arrow\cpp\src\arrow\vendored\uriparser\UriCompare.c(72): warning C4273: 'uriEqualsUriA': inconsistent dll linkage
C:\Users\Alison\clones\arrow\cpp\src\arrow\vendored\uriparser\Uri.h(585): note: see previous definition of 'uriEqualsUriA'
C:\Users\Alison\clones\arrow\cpp\src\arrow\vendored\uriparser\UriCompare.c(72): warning C4273: 'uriEqualsUriW': inconsistent dll linkage
C:\Users\Alison\clones\arrow\cpp\src\arrow\vendored\uriparser\Uri.h(585): note: see previous definition of 'uriEqualsUriW'
[8/18] Compiling C object src/arrow/arrow.dll.p/vendored_uriparser_UriFile.c.obj
C:\Users\Alison\clones\arrow\cpp\src\arrow\vendored\uriparser\UriFile.c(218): warning C4273: 'uriUnixFilenameToUriStringA': inconsistent dll linkage
C:\Users\Alison\clones\arrow\cpp\src\arrow\vendored\uriparser\Uri.h(742): note: see previous definition of 'uriUnixFilenameToUriStringA'
C:\Users\Alison\clones\arrow\cpp\src\arrow\vendored\uriparser\UriFile.c(224): warning C4273: 'uriWindowsFilenameToUriStringA': inconsistent dll linkage
C:\Users\Alison\clones\arrow\cpp\src\arrow\vendored\uriparser\Uri.h(765): note: see previous definition of 'uriWindowsFilenameToUriStringA'
C:\Users\Alison\clones\arrow\cpp\src\arrow\vendored\uriparser\UriFile.c(230): warning C4273: 'uriUriStringToUnixFilenameA': inconsistent dll linkage
C:\Users\Alison\clones\arrow\cpp\src\arrow\vendored\uriparser\Uri.h(784): note: see previous definition of 'uriUriStringToUnixFilenameA'
C:\Users\Alison\clones\arrow\cpp\src\arrow\vendored\uriparser\UriFile.c(236): warning C4273: 'uriUriStringToWindowsFilenameA': inconsistent dll linkage
C:\Users\Alison\clones\arrow\cpp\src\arrow\vendored\uriparser\Uri.h(803): note: see previous definition of 'uriUriStringToWindowsFilenameA'
C:\Users\Alison\clones\arrow\cpp\src\arrow\vendored\uriparser\UriFile.c(218): warning C4273: 'uriUnixFilenameToUriStringW': inconsistent dll linkage
C:\Users\Alison\clones\arrow\cpp\src\arrow\vendored\uriparser\Uri.h(742): note: see previous definition of 'uriUnixFilenameToUriStringW'
C:\Users\Alison\clones\arrow\cpp\src\arrow\vendored\uriparser\UriFile.c(224): warning C4273: 'uriWindowsFilenameToUriStringW': inconsistent dll linkage
C:\Users\Alison\clones\arrow\cpp\src\arrow\vendored\uriparser\Uri.h(765): note: see previous definition of 'uriWindowsFilenameToUriStringW'
C:\Users\Alison\clones\arrow\cpp\src\arrow\vendored\uriparser\UriFile.c(230): warning C4273: 'uriUriStringToUnixFilenameW': inconsistent dll linkage
C:\Users\Alison\clones\arrow\cpp\src\arrow\vendored\uriparser\Uri.h(784): note: see previous definition of 'uriUriStringToUnixFilenameW'
C:\Users\Alison\clones\arrow\cpp\src\arrow\vendored\uriparser\UriFile.c(236): warning C4273: 'uriUriStringToWindowsFilenameW': inconsistent dll linkage
C:\Users\Alison\clones\arrow\cpp\src\arrow\vendored\uriparser\Uri.h(803): note: see previous definition of 'uriUriStringToWindowsFilenameW'
[10/18] Compiling C object src/arrow/arrow.dll.p/vendored_uriparser_UriEscape.c.obj
C:\Users\Alison\clones\arrow\cpp\src\arrow\vendored\uriparser\UriEscape.c(71): warning C4273: 'uriEscapeA': inconsistent dll linkage
C:\Users\Alison\clones\arrow\cpp\src\arrow\vendored\uriparser\Uri.h(395): note: see previous definition of 'uriEscapeA'
C:\Users\Alison\clones\arrow\cpp\src\arrow\vendored\uriparser\UriEscape.c(78): warning C4273: 'uriEscapeExA': inconsistent dll linkage
C:\Users\Alison\clones\arrow\cpp\src\arrow\vendored\uriparser\Uri.h(371): note: see previous definition of 'uriEscapeExA'
C:\Users\Alison\clones\arrow\cpp\src\arrow\vendored\uriparser\UriEscape.c(247): warning C4273: 'uriUnescapeInPlaceA': inconsistent dll linkage
C:\Users\Alison\clones\arrow\cpp\src\arrow\vendored\uriparser\Uri.h(440): note: see previous definition of 'uriUnescapeInPlaceA'
C:\Users\Alison\clones\arrow\cpp\src\arrow\vendored\uriparser\UriEscape.c(253): warning C4273: 'uriUnescapeInPlaceExA': inconsistent dll linkage
C:\Users\Alison\clones\arrow\cpp\src\arrow\vendored\uriparser\Uri.h(417): note: see previous definition of 'uriUnescapeInPlaceExA'
C:\Users\Alison\clones\arrow\cpp\src\arrow\vendored\uriparser\UriEscape.c(71): warning C4273: 'uriEscapeW': inconsistent dll linkage
C:\Users\Alison\clones\arrow\cpp\src\arrow\vendored\uriparser\Uri.h(395): note: see previous definition of 'uriEscapeW'
C:\Users\Alison\clones\arrow\cpp\src\arrow\vendored\uriparser\UriEscape.c(78): warning C4273: 'uriEscapeExW': inconsistent dll linkage
C:\Users\Alison\clones\arrow\cpp\src\arrow\vendored\uriparser\Uri.h(371): note: see previous definition of 'uriEscapeExW'
C:\Users\Alison\clones\arrow\cpp\src\arrow\vendored\uriparser\UriEscape.c(247): warning C4273: 'uriUnescapeInPlaceW': inconsistent dll linkage
C:\Users\Alison\clones\arrow\cpp\src\arrow\vendored\uriparser\Uri.h(440): note: see previous definition of 'uriUnescapeInPlaceW'
C:\Users\Alison\clones\arrow\cpp\src\arrow\vendored\uriparser\UriEscape.c(253): warning C4273: 'uriUnescapeInPlaceExW': inconsistent dll linkage
C:\Users\Alison\clones\arrow\cpp\src\arrow\vendored\uriparser\Uri.h(417): note: see previous definition of 'uriUnescapeInPlaceExW'
[11/18] Compiling C object src/arrow/arrow.dll.p/vendored_uriparser_UriNormalize.c.obj
C:\Users\Alison\clones\arrow\cpp\src\arrow\vendored\uriparser\UriNormalize.c(477): warning C4273: 'uriNormalizeSyntaxMaskRequiredA': inconsistent dll linkage
C:\Users\Alison\clones\arrow\cpp\src\arrow\vendored\uriparser\Uri.h(638): note: see previous definition of 'uriNormalizeSyntaxMaskRequiredA'
C:\Users\Alison\clones\arrow\cpp\src\arrow\vendored\uriparser\UriNormalize.c(485): warning C4273: 'uriNormalizeSyntaxMaskRequiredExA': inconsistent dll linkage
C:\Users\Alison\clones\arrow\cpp\src\arrow\vendored\uriparser\Uri.h(656): note: see previous definition of 'uriNormalizeSyntaxMaskRequiredExA'
C:\Users\Alison\clones\arrow\cpp\src\arrow\vendored\uriparser\UriNormalize.c(512): warning C4273: 'uriNormalizeSyntaxExA': inconsistent dll linkage
C:\Users\Alison\clones\arrow\cpp\src\arrow\vendored\uriparser\Uri.h(678): note: see previous definition of 'uriNormalizeSyntaxExA'
C:\Users\Alison\clones\arrow\cpp\src\arrow\vendored\uriparser\UriNormalize.c(518): warning C4273: 'uriNormalizeSyntaxExMmA': inconsistent dll linkage
C:\Users\Alison\clones\arrow\cpp\src\arrow\vendored\uriparser\Uri.h(700): note: see previous definition of 'uriNormalizeSyntaxExMmA'
C:\Users\Alison\clones\arrow\cpp\src\arrow\vendored\uriparser\UriNormalize.c(526): warning C4273: 'uriNormalizeSyntaxA': inconsistent dll linkage
C:\Users\Alison\clones\arrow\cpp\src\arrow\vendored\uriparser\Uri.h(720): note: see previous definition of 'uriNormalizeSyntaxA'
C:\Users\Alison\clones\arrow\cpp\src\arrow\vendored\uriparser\UriNormalize.c(477): warning C4273: 'uriNormalizeSyntaxMaskRequiredW': inconsistent dll linkage
C:\Users\Alison\clones\arrow\cpp\src\arrow\vendored\uriparser\Uri.h(638): note: see previous definition of 'uriNormalizeSyntaxMaskRequiredW'
C:\Users\Alison\clones\arrow\cpp\src\arrow\vendored\uriparser\UriNormalize.c(485): warning C4273: 'uriNormalizeSyntaxMaskRequiredExW': inconsistent dll linkage
C:\Users\Alison\clones\arrow\cpp\src\arrow\vendored\uriparser\Uri.h(656): note: see previous definition of 'uriNormalizeSyntaxMaskRequiredExW'
C:\Users\Alison\clones\arrow\cpp\src\arrow\vendored\uriparser\UriNormalize.c(512): warning C4273: 'uriNormalizeSyntaxExW': inconsistent dll linkage
C:\Users\Alison\clones\arrow\cpp\src\arrow\vendored\uriparser\Uri.h(678): note: see previous definition of 'uriNormalizeSyntaxExW'
C:\Users\Alison\clones\arrow\cpp\src\arrow\vendored\uriparser\UriNormalize.c(518): warning C4273: 'uriNormalizeSyntaxExMmW': inconsistent dll linkage
C:\Users\Alison\clones\arrow\cpp\src\arrow\vendored\uriparser\Uri.h(700): note: see previous definition of 'uriNormalizeSyntaxExMmW'
C:\Users\Alison\clones\arrow\cpp\src\arrow\vendored\uriparser\UriNormalize.c(526): warning C4273: 'uriNormalizeSyntaxW': inconsistent dll linkage
C:\Users\Alison\clones\arrow\cpp\src\arrow\vendored\uriparser\Uri.h(720): note: see previous definition of 'uriNormalizeSyntaxW'
[12/18] Compiling C object src/arrow/arrow.dll.p/vendored_uriparser_UriQuery.c.obj
C:\Users\Alison\clones\arrow\cpp\src\arrow\vendored\uriparser\UriQuery.c(89): warning C4273: 'uriComposeQueryCharsRequiredA': inconsistent dll linkage
C:\Users\Alison\clones\arrow\cpp\src\arrow\vendored\uriparser\Uri.h(822): note: see previous definition of 'uriComposeQueryCharsRequiredA'
C:\Users\Alison\clones\arrow\cpp\src\arrow\vendored\uriparser\UriQuery.c(100): warning C4273: 'uriComposeQueryCharsRequiredExA': inconsistent dll linkage
C:\Users\Alison\clones\arrow\cpp\src\arrow\vendored\uriparser\Uri.h(842): note: see previous definition of 'uriComposeQueryCharsRequiredExA'
C:\Users\Alison\clones\arrow\cpp\src\arrow\vendored\uriparser\UriQuery.c(112): warning C4273: 'uriComposeQueryA': inconsistent dll linkage
C:\Users\Alison\clones\arrow\cpp\src\arrow\vendored\uriparser\Uri.h(870): note: see previous definition of 'uriComposeQueryA'
C:\Users\Alison\clones\arrow\cpp\src\arrow\vendored\uriparser\UriQuery.c(123): warning C4273: 'uriComposeQueryExA': inconsistent dll linkage
C:\Users\Alison\clones\arrow\cpp\src\arrow\vendored\uriparser\Uri.h(897): note: see previous definition of 'uriComposeQueryExA'
C:\Users\Alison\clones\arrow\cpp\src\arrow\vendored\uriparser\UriQuery.c(140): warning C4273: 'uriComposeQueryMallocA': inconsistent dll linkage
C:\Users\Alison\clones\arrow\cpp\src\arrow\vendored\uriparser\Uri.h(923): note: see previous definition of 'uriComposeQueryMallocA'
C:\Users\Alison\clones\arrow\cpp\src\arrow\vendored\uriparser\UriQuery.c(151): warning C4273: 'uriComposeQueryMallocExA': inconsistent dll linkage
C:\Users\Alison\clones\arrow\cpp\src\arrow\vendored\uriparser\Uri.h(948): note: see previous definition of 'uriComposeQueryMallocExA'
C:\Users\Alison\clones\arrow\cpp\src\arrow\vendored\uriparser\UriQuery.c(160): warning C4273: 'uriComposeQueryMallocExMmA': inconsistent dll linkage
C:\Users\Alison\clones\arrow\cpp\src\arrow\vendored\uriparser\Uri.h(974): note: see previous definition of 'uriComposeQueryMallocExMmA'
C:\Users\Alison\clones\arrow\cpp\src\arrow\vendored\uriparser\UriQuery.c(360): warning C4273: 'uriFreeQueryListA': inconsistent dll linkage
C:\Users\Alison\clones\arrow\cpp\src\arrow\vendored\uriparser\Uri.h(1063): note: see previous definition of 'uriFreeQueryListA'
C:\Users\Alison\clones\arrow\cpp\src\arrow\vendored\uriparser\UriQuery.c(366): warning C4273: 'uriFreeQueryListMmA': inconsistent dll linkage
C:\Users\Alison\clones\arrow\cpp\src\arrow\vendored\uriparser\Uri.h(1078): note: see previous definition of 'uriFreeQueryListMmA'
C:\Users\Alison\clones\arrow\cpp\src\arrow\vendored\uriparser\UriQuery.c(381): warning C4273: 'uriDissectQueryMallocA': inconsistent dll linkage
C:\Users\Alison\clones\arrow\cpp\src\arrow\vendored\uriparser\Uri.h(999): note: see previous definition of 'uriDissectQueryMallocA'
C:\Users\Alison\clones\arrow\cpp\src\arrow\vendored\uriparser\UriQuery.c(392): warning C4273: 'uriDissectQueryMallocExA': inconsistent dll linkage
C:\Users\Alison\clones\arrow\cpp\src\arrow\vendored\uriparser\Uri.h(1022): note: see previous definition of 'uriDissectQueryMallocExA'
C:\Users\Alison\clones\arrow\cpp\src\arrow\vendored\uriparser\UriQuery.c(401): warning C4273: 'uriDissectQueryMallocExMmA': inconsistent dll linkage
C:\Users\Alison\clones\arrow\cpp\src\arrow\vendored\uriparser\Uri.h(1047): note: see previous definition of 'uriDissectQueryMallocExMmA'
C:\Users\Alison\clones\arrow\cpp\src\arrow\vendored\uriparser\UriQuery.c(89): warning C4273: 'uriComposeQueryCharsRequiredW': inconsistent dll linkage
C:\Users\Alison\clones\arrow\cpp\src\arrow\vendored\uriparser\Uri.h(822): note: see previous definition of 'uriComposeQueryCharsRequiredW'
C:\Users\Alison\clones\arrow\cpp\src\arrow\vendored\uriparser\UriQuery.c(100): warning C4273: 'uriComposeQueryCharsRequiredExW': inconsistent dll linkage
C:\Users\Alison\clones\arrow\cpp\src\arrow\vendored\uriparser\Uri.h(842): note: see previous definition of 'uriComposeQueryCharsRequiredExW'
C:\Users\Alison\clones\arrow\cpp\src\arrow\vendored\uriparser\UriQuery.c(112): warning C4273: 'uriComposeQueryW': inconsistent dll linkage
C:\Users\Alison\clones\arrow\cpp\src\arrow\vendored\uriparser\Uri.h(870): note: see previous definition of 'uriComposeQueryW'
C:\Users\Alison\clones\arrow\cpp\src\arrow\vendored\uriparser\UriQuery.c(123): warning C4273: 'uriComposeQueryExW': inconsistent dll linkage
C:\Users\Alison\clones\arrow\cpp\src\arrow\vendored\uriparser\Uri.h(897): note: see previous definition of 'uriComposeQueryExW'
C:\Users\Alison\clones\arrow\cpp\src\arrow\vendored\uriparser\UriQuery.c(140): warning C4273: 'uriComposeQueryMallocW': inconsistent dll linkage
C:\Users\Alison\clones\arrow\cpp\src\arrow\vendored\uriparser\Uri.h(923): note: see previous definition of 'uriComposeQueryMallocW'
C:\Users\Alison\clones\arrow\cpp\src\arrow\vendored\uriparser\UriQuery.c(151): warning C4273: 'uriComposeQueryMallocExW': inconsistent dll linkage
C:\Users\Alison\clones\arrow\cpp\src\arrow\vendored\uriparser\Uri.h(948): note: see previous definition of 'uriComposeQueryMallocExW'
C:\Users\Alison\clones\arrow\cpp\src\arrow\vendored\uriparser\UriQuery.c(160): warning C4273: 'uriComposeQueryMallocExMmW': inconsistent dll linkage
C:\Users\Alison\clones\arrow\cpp\src\arrow\vendored\uriparser\Uri.h(974): note: see previous definition of 'uriComposeQueryMallocExMmW'
C:\Users\Alison\clones\arrow\cpp\src\arrow\vendored\uriparser\UriQuery.c(360): warning C4273: 'uriFreeQueryListW': inconsistent dll linkage
C:\Users\Alison\clones\arrow\cpp\src\arrow\vendored\uriparser\Uri.h(1063): note: see previous definition of 'uriFreeQueryListW'
C:\Users\Alison\clones\arrow\cpp\src\arrow\vendored\uriparser\UriQuery.c(366): warning C4273: 'uriFreeQueryListMmW': inconsistent dll linkage
C:\Users\Alison\clones\arrow\cpp\src\arrow\vendored\uriparser\Uri.h(1078): note: see previous definition of 'uriFreeQueryListMmW'
C:\Users\Alison\clones\arrow\cpp\src\arrow\vendored\uriparser\UriQuery.c(381): warning C4273: 'uriDissectQueryMallocW': inconsistent dll linkage
C:\Users\Alison\clones\arrow\cpp\src\arrow\vendored\uriparser\Uri.h(999): note: see previous definition of 'uriDissectQueryMallocW'
C:\Users\Alison\clones\arrow\cpp\src\arrow\vendored\uriparser\UriQuery.c(392): warning C4273: 'uriDissectQueryMallocExW': inconsistent dll linkage
C:\Users\Alison\clones\arrow\cpp\src\arrow\vendored\uriparser\Uri.h(1022): note: see previous definition of 'uriDissectQueryMallocExW'
C:\Users\Alison\clones\arrow\cpp\src\arrow\vendored\uriparser\UriQuery.c(401): warning C4273: 'uriDissectQueryMallocExMmW': inconsistent dll linkage
C:\Users\Alison\clones\arrow\cpp\src\arrow\vendored\uriparser\Uri.h(1047): note: see previous definition of 'uriDissectQueryMallocExMmW'
[13/18] Compiling C object src/arrow/arrow.dll.p/vendored_uriparser_UriRecompose.c.obj
C:\Users\Alison\clones\arrow\cpp\src\arrow\vendored\uriparser\UriRecompose.c(76): warning C4273: 'uriToStringCharsRequiredA': inconsistent dll linkage
C:\Users\Alison\clones\arrow\cpp\src\arrow\vendored\uriparser\Uri.h(602): note: see previous definition of 'uriToStringCharsRequiredA'
C:\Users\Alison\clones\arrow\cpp\src\arrow\vendored\uriparser\UriRecompose.c(84): warning C4273: 'uriToStringA': inconsistent dll linkage
C:\Users\Alison\clones\arrow\cpp\src\arrow\vendored\uriparser\Uri.h(620): note: see previous definition of 'uriToStringA'
C:\Users\Alison\clones\arrow\cpp\src\arrow\vendored\uriparser\UriRecompose.c(76): warning C4273: 'uriToStringCharsRequiredW': inconsistent dll linkage
C:\Users\Alison\clones\arrow\cpp\src\arrow\vendored\uriparser\Uri.h(602): note: see previous definition of 'uriToStringCharsRequiredW'
C:\Users\Alison\clones\arrow\cpp\src\arrow\vendored\uriparser\UriRecompose.c(84): warning C4273: 'uriToStringW': inconsistent dll linkage
C:\Users\Alison\clones\arrow\cpp\src\arrow\vendored\uriparser\Uri.h(620): note: see previous definition of 'uriToStringW'
[14/18] Compiling C object src/arrow/arrow.dll.p/vendored_uriparser_UriShorten.c.obj
C:\Users\Alison\clones\arrow\cpp\src\arrow\vendored\uriparser\UriShorten.c(296): warning C4273: 'uriRemoveBaseUriA': inconsistent dll linkage
C:\Users\Alison\clones\arrow\cpp\src\arrow\vendored\uriparser\Uri.h(538): note: see previous definition of 'uriRemoveBaseUriA'
C:\Users\Alison\clones\arrow\cpp\src\arrow\vendored\uriparser\UriShorten.c(306): warning C4273: 'uriRemoveBaseUriMmA': inconsistent dll linkage
C:\Users\Alison\clones\arrow\cpp\src\arrow\vendored\uriparser\Uri.h(567): note: see previous definition of 'uriRemoveBaseUriMmA'
C:\Users\Alison\clones\arrow\cpp\src\arrow\vendored\uriparser\UriShorten.c(296): warning C4273: 'uriRemoveBaseUriW': inconsistent dll linkage
C:\Users\Alison\clones\arrow\cpp\src\arrow\vendored\uriparser\Uri.h(538): note: see previous definition of 'uriRemoveBaseUriW'
C:\Users\Alison\clones\arrow\cpp\src\arrow\vendored\uriparser\UriShorten.c(306): warning C4273: 'uriRemoveBaseUriMmW': inconsistent dll linkage
C:\Users\Alison\clones\arrow\cpp\src\arrow\vendored\uriparser\Uri.h(567): note: see previous definition of 'uriRemoveBaseUriMmW'
[15/18] Compiling C object src/arrow/arrow.dll.p/vendored_uriparser_UriResolve.c.obj
C:\Users\Alison\clones\arrow\cpp\src\arrow\vendored\uriparser\UriResolve.c(297): warning C4273: 'uriAddBaseUriA': inconsistent dll linkage
C:\Users\Alison\clones\arrow\cpp\src\arrow\vendored\uriparser\Uri.h(461): note: see previous definition of 'uriAddBaseUriA'
C:\Users\Alison\clones\arrow\cpp\src\arrow\vendored\uriparser\UriResolve.c(305): warning C4273: 'uriAddBaseUriExA': inconsistent dll linkage
C:\Users\Alison\clones\arrow\cpp\src\arrow\vendored\uriparser\Uri.h(484): note: see previous definition of 'uriAddBaseUriExA'
C:\Users\Alison\clones\arrow\cpp\src\arrow\vendored\uriparser\UriResolve.c(313): warning C4273: 'uriAddBaseUriExMmA': inconsistent dll linkage
C:\Users\Alison\clones\arrow\cpp\src\arrow\vendored\uriparser\Uri.h(509): note: see previous definition of 'uriAddBaseUriExMmA'
C:\Users\Alison\clones\arrow\cpp\src\arrow\vendored\uriparser\UriResolve.c(297): warning C4273: 'uriAddBaseUriW': inconsistent dll linkage
C:\Users\Alison\clones\arrow\cpp\src\arrow\vendored\uriparser\Uri.h(461): note: see previous definition of 'uriAddBaseUriW'
C:\Users\Alison\clones\arrow\cpp\src\arrow\vendored\uriparser\UriResolve.c(305): warning C4273: 'uriAddBaseUriExW': inconsistent dll linkage
C:\Users\Alison\clones\arrow\cpp\src\arrow\vendored\uriparser\Uri.h(484): note: see previous definition of 'uriAddBaseUriExW'
C:\Users\Alison\clones\arrow\cpp\src\arrow\vendored\uriparser\UriResolve.c(313): warning C4273: 'uriAddBaseUriExMmW': inconsistent dll linkage
C:\Users\Alison\clones\arrow\cpp\src\arrow\vendored\uriparser\Uri.h(509): note: see previous definition of 'uriAddBaseUriExMmW'
[16/18] Compiling C object src/arrow/arrow.dll.p/vendored_uriparser_UriParse.c.obj
C:\Users\Alison\clones\arrow\cpp\src\arrow\vendored\uriparser\UriParse.c(2156): warning C4273: 'uriParseUriExA': inconsistent dll linkage
C:\Users\Alison\clones\arrow\cpp\src\arrow\vendored\uriparser\Uri.h(221): note: see previous definition of 'uriParseUriExA'
C:\Users\Alison\clones\arrow\cpp\src\arrow\vendored\uriparser\UriParse.c(2203): warning C4273: 'uriParseUriA': inconsistent dll linkage
C:\Users\Alison\clones\arrow\cpp\src\arrow\vendored\uriparser\Uri.h(241): note: see previous definition of 'uriParseUriA'
C:\Users\Alison\clones\arrow\cpp\src\arrow\vendored\uriparser\UriParse.c(2212): warning C4273: 'uriParseSingleUriA': inconsistent dll linkage
C:\Users\Alison\clones\arrow\cpp\src\arrow\vendored\uriparser\Uri.h(263): note: see previous definition of 'uriParseSingleUriA'
C:\Users\Alison\clones\arrow\cpp\src\arrow\vendored\uriparser\UriParse.c(2219): warning C4273: 'uriParseSingleUriExA': inconsistent dll linkage
C:\Users\Alison\clones\arrow\cpp\src\arrow\vendored\uriparser\Uri.h(288): note: see previous definition of 'uriParseSingleUriExA'
C:\Users\Alison\clones\arrow\cpp\src\arrow\vendored\uriparser\UriParse.c(2230): warning C4273: 'uriParseSingleUriExMmA': inconsistent dll linkage
C:\Users\Alison\clones\arrow\cpp\src\arrow\vendored\uriparser\Uri.h(314): note: see previous definition of 'uriParseSingleUriExMmA'
C:\Users\Alison\clones\arrow\cpp\src\arrow\vendored\uriparser\UriParse.c(2258): warning C4273: 'uriFreeUriMembersA': inconsistent dll linkage
C:\Users\Alison\clones\arrow\cpp\src\arrow\vendored\uriparser\Uri.h(331): note: see previous definition of 'uriFreeUriMembersA'
C:\Users\Alison\clones\arrow\cpp\src\arrow\vendored\uriparser\UriParse.c(2264): warning C4273: 'uriFreeUriMembersMmA': inconsistent dll linkage
C:\Users\Alison\clones\arrow\cpp\src\arrow\vendored\uriparser\Uri.h(347): note: see previous definition of 'uriFreeUriMembersMmA'
C:\Users\Alison\clones\arrow\cpp\src\arrow\vendored\uriparser\UriParse.c(2156): warning C4273: 'uriParseUriExW': inconsistent dll linkage
C:\Users\Alison\clones\arrow\cpp\src\arrow\vendored\uriparser\Uri.h(221): note: see previous definition of 'uriParseUriExW'
C:\Users\Alison\clones\arrow\cpp\src\arrow\vendored\uriparser\UriParse.c(2203): warning C4273: 'uriParseUriW': inconsistent dll linkage
C:\Users\Alison\clones\arrow\cpp\src\arrow\vendored\uriparser\Uri.h(241): note: see previous definition of 'uriParseUriW'
C:\Users\Alison\clones\arrow\cpp\src\arrow\vendored\uriparser\UriParse.c(2212): warning C4273: 'uriParseSingleUriW': inconsistent dll linkage
C:\Users\Alison\clones\arrow\cpp\src\arrow\vendored\uriparser\Uri.h(263): note: see previous definition of 'uriParseSingleUriW'
C:\Users\Alison\clones\arrow\cpp\src\arrow\vendored\uriparser\UriParse.c(2219): warning C4273: 'uriParseSingleUriExW': inconsistent dll linkage
C:\Users\Alison\clones\arrow\cpp\src\arrow\vendored\uriparser\Uri.h(288): note: see previous definition of 'uriParseSingleUriExW'
C:\Users\Alison\clones\arrow\cpp\src\arrow\vendored\uriparser\UriParse.c(2230): warning C4273: 'uriParseSingleUriExMmW': inconsistent dll linkage
C:\Users\Alison\clones\arrow\cpp\src\arrow\vendored\uriparser\Uri.h(314): note: see previous definition of 'uriParseSingleUriExMmW'
C:\Users\Alison\clones\arrow\cpp\src\arrow\vendored\uriparser\UriParse.c(2258): warning C4273: 'uriFreeUriMembersW': inconsistent dll linkage
C:\Users\Alison\clones\arrow\cpp\src\arrow\vendored\uriparser\Uri.h(331): note: see previous definition of 'uriFreeUriMembersW'
C:\Users\Alison\clones\arrow\cpp\src\arrow\vendored\uriparser\UriParse.c(2264): warning C4273: 'uriFreeUriMembersMmW': inconsistent dll linkage
C:\Users\Alison\clones\arrow\cpp\src\arrow\vendored\uriparser\Uri.h(347): note: see previous definition of 'uriFreeUriMembersMmW'
[17/18] Linking target src/arrow/arrow.dll
Creating library src/arrow\arrow.lib and object src/arrow\arrow.exp
LINK : warning LNK4217: symbol 'uriEscapeExA' defined in 'vendored_uriparser_UriEscape.c.obj' is imported by 'vendored_uriparser_UriFile.c.obj' in function 'uriFilenameToUriStringA'
LINK : warning LNK4286: symbol 'uriEscapeExA' defined in 'vendored_uriparser_UriEscape.c.obj' is imported by 'vendored_uriparser_UriQuery.c.obj'
LINK : warning LNK4217: symbol 'uriUnescapeInPlaceExA' defined in 'vendored_uriparser_UriEscape.c.obj' is imported by 'vendored_uriparser_UriFile.c.obj' in function 'uriUriStringToFilenameA'
LINK : warning LNK4286: symbol 'uriUnescapeInPlaceExA' defined in 'vendored_uriparser_UriEscape.c.obj' is imported by 'vendored_uriparser_UriQuery.c.obj'
LINK : warning LNK4217: symbol 'uriEscapeExW' defined in 'vendored_uriparser_UriEscape.c.obj' is imported by 'vendored_uriparser_UriFile.c.obj' in function 'uriFilenameToUriStringW'
LINK : warning LNK4286: symbol 'uriEscapeExW' defined in 'vendored_uriparser_UriEscape.c.obj' is imported by 'vendored_uriparser_UriQuery.c.obj'
LINK : warning LNK4217: symbol 'uriUnescapeInPlaceExW' defined in 'vendored_uriparser_UriEscape.c.obj' is imported by 'vendored_uriparser_UriFile.c.obj' in function 'uriUriStringToFilenameW'
LINK : warning LNK4286: symbol 'uriUnescapeInPlaceExW' defined in 'vendored_uriparser_UriEscape.c.obj' is imported by 'vendored_uriparser_UriQuery.c.obj'
LINK : warning LNK4217: symbol 'uriParseIpFourAddressA' defined in 'vendored_uriparser_UriIp4.c.obj' is imported by 'vendored_uriparser_UriParse.c.obj' in function 'uriOnExitOwnHost2A'
LINK : warning LNK4217: symbol 'uriParseIpFourAddressW' defined in 'vendored_uriparser_UriIp4.c.obj' is imported by 'vendored_uriparser_UriParse.c.obj' in function 'uriOnExitOwnHost2W'
LINK : warning LNK4217: symbol 'uriFreeUriMembersMmA' defined in 'vendored_uriparser_UriParse.c.obj' is imported by 'vendored_uriparser_UriResolve.c.obj' in function 'uriAddBaseUriExMmA'
LINK : warning LNK4217: symbol 'uriFreeUriMembersMmA' defined in 'vendored_uriparser_UriParse.c.obj' is imported by 'vendored_uriparser_UriShorten.c.obj' in function 'uriEqualsAuthorityA'
LINK : warning LNK4217: symbol 'uriFreeUriMembersMmW' defined in 'vendored_uriparser_UriParse.c.obj' is imported by 'vendored_uriparser_UriResolve.c.obj' in function 'uriAddBaseUriExMmW'
LINK : warning LNK4217: symbol 'uriFreeUriMembersMmW' defined in 'vendored_uriparser_UriParse.c.obj' is imported by 'vendored_uriparser_UriShorten.c.obj' in function 'uriRemoveBaseUriImplW'
```
</details>
I have a poor understanding of the MSVC linker, so I can't answer with much detail why. Maybe we should just conditionally define this on MSVC instead of all the time?
--
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: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] GH-46797: [C++] Support Meson on Windows [arrow]
Posted by "WillAyd (via GitHub)" <gi...@apache.org>.
WillAyd commented on code in PR #47282:
URL: https://github.com/apache/arrow/pull/47282#discussion_r2341297949
##########
cpp/src/arrow/meson.build:
##########
@@ -420,19 +481,38 @@ foreach key, val : arrow_components
arrow_deps += val.get('dependencies', [])
endforeach
+arrow_lib_cpp_shared_args = ['-DARROW_EXPORTING', '-DURI_LIBRARY_BUILD']
+if needs_compute
+ arrow_lib_cpp_shared_args += ['-DARROW_COMPUTE_EXPORTING']
+endif
+
arrow_lib = library(
'arrow',
sources: arrow_srcs,
include_directories: arrow_includes,
dependencies: arrow_deps,
install: true,
gnu_symbol_visibility: 'inlineshidden',
- cpp_shared_args: ['-DARROW_EXPORTING'],
+ gnu_symbol_visibility: 'hidden',
+ c_shared_args: [
+ '-DARROW_EXPORTING',
+ '-DURI_LIBRARY_BUILD',
+ '-DURI_VISIBILITY',
Review Comment:
Ah OK I think I understand the intent better now - thanks! The latest updates should reflect this
--
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: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] GH-46797: [C++] Support Meson on Windows [arrow]
Posted by "WillAyd (via GitHub)" <gi...@apache.org>.
WillAyd commented on PR #47282:
URL: https://github.com/apache/arrow/pull/47282#issuecomment-3202581921
I don't think the glib & ruby failure is related
--
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: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] GH-46797: [C++] Support Meson on Windows [arrow]
Posted by "kou (via GitHub)" <gi...@apache.org>.
kou commented on code in PR #47282:
URL: https://github.com/apache/arrow/pull/47282#discussion_r2264567627
##########
.github/workflows/cpp_extra.yml:
##########
@@ -166,3 +166,73 @@ jobs:
ARCHERY_DOCKER_PASSWORD: ${{ secrets.DOCKERHUB_TOKEN }}
continue-on-error: true
run: archery docker push ${{ matrix.image }}
+
+ meson-windows:
+ needs: check-labels
+ name: ${{ matrix.title }}
+ runs-on: ${{ matrix.runs-on }}
+ if: needs.check-labels.outputs.ci-extra == 'true'
+ timeout-minutes: 75
+ strategy:
+ fail-fast: false
+ matrix:
+ include:
+ - runs-on: windows-2022
+ title: AMD64 Windows Meson
+ steps:
+ - name: Checkout Arrow
+ uses: actions/checkout@v4
+ with:
+ fetch-depth: 0
+ submodules: recursive
+ - name: Download Timezone Database
+ shell: bash
+ run: ci/scripts/download_tz_database.sh
+ - 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: cpp-ccache-windows-meson-${{ env.CACHE_VERSION }}-${{ hashFiles('cpp/**') }}
+ restore-keys: cpp-ccache-windows-meson-${{ env.CACHE_VERSION }}-
+ env:
+ # We can invalidate the current cache by updating this.
+ CACHE_VERSION: "2022-09-13"
+ - name: Setup Python
+ uses: actions/setup-python@a26af69be951a213d495a4c3e4e4022e16d87065 # v5.6.0
+ with:
+ python-version: 3
+ - name: Install Meson
+ run: |
+ python3 -m pip install meson
+ - name: Build
+ shell: cmd
+ env:
+ ARROW_USE_MESON: ON
+ ARROW_HOME: C:\arrow-dist
+ run: |
+ echo %PATH%
+ call "C:\Program Files\Microsoft Visual Studio\2022\Enterprise\VC\Auxiliary\Build\vcvarsall.bat" x64
+ echo %PATH%
+ dir "C:\Program Files\Microsoft Visual Studio\2022\Enterprise\VC\Tools\MSVC\14.44.35207\bin\HostX64\x64"
+ bash -c "ci/scripts/cpp_build.sh $(pwd) $(pwd)/build ${{ matrix.run-options || '' }}"
Review Comment:
How about trying `meson setup --vsenv` instead of calling `vcvarsall.bat` manually?
https://mesonbuild.com/Using-with-Visual-Studio.html
> If no Visual Studio Command Prompt was detected, and no mingw compilers are detected either, meson will attempt to find "a" Visual Studio installation for you automatically, by asking Microsoft's "vswhere" program. If you want to ignore mingw compilers, pass the --vsenv option on the meson command line. If you need to guarantee a specific Visual Studio version, set it up manually.
--
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: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] GH-46797: [C++] Support Meson on Windows [arrow]
Posted by "kou (via GitHub)" <gi...@apache.org>.
kou commented on code in PR #47282:
URL: https://github.com/apache/arrow/pull/47282#discussion_r2289698449
##########
.github/workflows/cpp_extra.yml:
##########
@@ -278,3 +279,70 @@ jobs:
cmake --build cpp/examples/minimal_build.build
cd cpp/examples/minimal_build
../minimal_build.build/arrow-example
+
+ meson-windows:
+ needs: check-labels
+ name: ${{ matrix.title }}
+ runs-on: ${{ matrix.runs-on }}
+ if: needs.check-labels.outputs.ci-extra == 'true'
+ timeout-minutes: 75
+ strategy:
+ fail-fast: false
+ matrix:
+ include:
+ - runs-on: windows-2022
+ title: AMD64 Windows Meson
+ steps:
+ - name: Checkout Arrow
+ uses: actions/checkout@v4
+ with:
+ fetch-depth: 0
+ submodules: recursive
+ - name: Download Timezone Database
+ shell: bash
+ run: ci/scripts/download_tz_database.sh
+ - 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: cpp-ccache-windows-meson-${{ env.CACHE_VERSION }}-${{ hashFiles('cpp/**') }}
+ restore-keys: cpp-ccache-windows-meson-${{ env.CACHE_VERSION }}-
+ env:
+ # We can invalidate the current cache by updating this.
+ CACHE_VERSION: "2022-09-13"
Review Comment:
Could you update this date?
##########
cpp/src/arrow/compute/CMakeLists.txt:
##########
@@ -52,6 +52,7 @@ if(ARROW_TESTING AND ARROW_COMPUTE)
target_link_libraries(arrow_compute_testing
PUBLIC $<TARGET_OBJECTS:arrow_compute_core_testing>
PUBLIC ${ARROW_GTEST_GTEST_MAIN})
+ target_compile_definitions(arrow_compute_testing PRIVATE COMPILED_BY_CMAKE)
Review Comment:
Hmm. What is the difference between Meson build and CMake build? GTest version?
##########
cpp/src/arrow/flight/meson.build:
##########
@@ -44,7 +44,17 @@ install_headers(
subdir: 'arrow/flight',
)
-grpc_dep = dependency('grpc++')
+# We specifically need to use the grpc subproject and cannot use a system installion
+# due to https://github.com/mesonbuild/meson/issues/14905
Review Comment:
Can we use https://mesonbuild.com/Reference-manual_returned_external_program.html#external_programfull_path as workaround?
If we can use `grpc_cpp_plugin.full_path()`, can we keep supporting system gRPC?
##########
ci/scripts/cpp_test.sh:
##########
@@ -95,6 +95,8 @@ if [ "${ARROW_USE_MESON:-OFF}" = "ON" ]; then
--no-rebuild \
--print-errorlogs \
--suite arrow \
+ --max-lines=0 \
+ --timeout-multiplier 10 \
Review Comment:
Can we unify `--XXX=V` and `-XXX V` style?
##########
cpp/src/arrow/flight/flight_test.cc:
##########
@@ -207,6 +207,9 @@ ARROW_FLIGHT_TEST_ASYNC_CLIENT(GrpcAsyncClientTest);
// Ad-hoc gRPC-specific tests
TEST(TestFlight, ConnectUri) {
+#ifndef ARROW_ENABLE_THREADING
+ GTEST_SKIP() << "This test requires Boost.Process to run";
+#endif
Review Comment:
How about adding `arrow::testing::Process::IsAvailable()` static method or something and use it here?
--
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: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] GH-46797: [C++] Support Meson on Windows [arrow]
Posted by "WillAyd (via GitHub)" <gi...@apache.org>.
WillAyd commented on code in PR #47282:
URL: https://github.com/apache/arrow/pull/47282#discussion_r2262998014
##########
cpp/src/arrow/compute/kernels/ree_util_internal.h:
##########
@@ -347,7 +347,7 @@ Result<std::shared_ptr<ArrayData>> PreallocateRunEndsArray(
/// \param has_validity_buffer a validity buffer must be allocated
/// \param length the length of the values array
/// \param data_buffer_size the size of the data buffer for string and binary types
-ARROW_COMPUTE_EXPORT Result<std::shared_ptr<ArrayData>> PreallocateValuesArray(
+ARROW_EXPORT Result<std::shared_ptr<ArrayData>> PreallocateValuesArray(
Review Comment:
Ah you are right - I can fix this
--
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: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Meson win support [arrow]
Posted by "WillAyd (via GitHub)" <gi...@apache.org>.
WillAyd commented on code in PR #47282:
URL: https://github.com/apache/arrow/pull/47282#discussion_r2261319860
##########
cpp/src/arrow/compute/kernels/ree_util_internal.h:
##########
@@ -347,7 +347,7 @@ Result<std::shared_ptr<ArrayData>> PreallocateRunEndsArray(
/// \param has_validity_buffer a validity buffer must be allocated
/// \param length the length of the values array
/// \param data_buffer_size the size of the data buffer for string and binary types
-ARROW_COMPUTE_EXPORT Result<std::shared_ptr<ArrayData>> PreallocateValuesArray(
+ARROW_EXPORT Result<std::shared_ptr<ArrayData>> PreallocateValuesArray(
Review Comment:
Although this file is in the compute/kernels tree, the associated module is built as part of the arrow lib irrespective of whether compute is enabled or not. As such, I figured it made more sense to use the EXPORT semantics from the arrow lib rather than arrow_compute
##########
cpp/src/arrow/util/meson.build:
##########
@@ -224,18 +222,25 @@ utility_test_srcs = [
]
if host_machine.system() == 'windows'
+ # meson does not natively support manifest files like CMake does
Review Comment:
I don't think the Meson support for manifest files is as nice as CMake's, but I'm also not super familiar with them. Need to research this a bit more...
##########
cpp/src/arrow/compute/CMakeLists.txt:
##########
@@ -52,6 +52,7 @@ if(ARROW_TESTING AND ARROW_COMPUTE)
target_link_libraries(arrow_compute_testing
PUBLIC $<TARGET_OBJECTS:arrow_compute_core_testing>
PUBLIC ${ARROW_GTEST_GTEST_MAIN})
+ target_compile_definitions(arrow_compute_testing PRIVATE COMPILED_BY_CMAKE)
Review Comment:
Not a huge fan of this, but there appears to be an issue with the registry on MSVC with the Meson configuration that isn't in the CMake config.
I think this has to do with the CMake config forcing a certain type of linkage, but I'm not sure yet. I am also under the impression that the Meson/non-MSVC path is preferred, so we might prefer to "fix" CMake rather than make the Meson configuration match it
--
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: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] GH-46797: [C++] Support Meson on Windows [arrow]
Posted by "lidavidm (via GitHub)" <gi...@apache.org>.
lidavidm commented on PR #47282:
URL: https://github.com/apache/arrow/pull/47282#issuecomment-3186182727
```
Listing only the last 100 lines from a long log.
```
There's an actual failing test (`TestFlight.ConnectUri`), but Meson is omitting the important part. Is there any way to get the full output? The stderr output is irrelevant, I think: there are tests that intentionally misconfigure TLS to make sure things behave as expected (and gRPC spams stderr apparently).
--
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: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] GH-46797: [C++] Support Meson on Windows [arrow]
Posted by "lidavidm (via GitHub)" <gi...@apache.org>.
lidavidm commented on PR #47282:
URL: https://github.com/apache/arrow/pull/47282#issuecomment-3181503335
@WillAyd where in the logs is this? I see compilation failures on the current state of the branch
That "error" is _probably_ a red herring and I'll note that gRPC is segfaulty on Windows (e.g. https://github.com/grpc/grpc/issues/29185)
--
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: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] GH-46797: [C++] Support Meson on Windows [arrow]
Posted by "WillAyd (via GitHub)" <gi...@apache.org>.
WillAyd commented on code in PR #47282:
URL: https://github.com/apache/arrow/pull/47282#discussion_r2304121198
##########
cpp/src/arrow/testing/gtest_util_test.cc:
##########
@@ -287,9 +287,11 @@ TEST(TestWithinUlp, Float) {
TEST(AssertTestWithinUlp, Basics) {
AssertWithinUlp(123.4567, 123.45670000000015, 11);
AssertWithinUlp(123.456f, 123.456085f, 11);
+#if !defined(_MSC_VER)
Review Comment:
I've also opened https://github.com/apache/arrow/issues/47442 to track the behavior of this particular test
--
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: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] GH-46797: [C++] Support Meson on Windows [arrow]
Posted by "WillAyd (via GitHub)" <gi...@apache.org>.
WillAyd commented on PR #47282:
URL: https://github.com/apache/arrow/pull/47282#issuecomment-3258192387
https://github.com/apache/arrow/pull/47455 should help clean up some of the testing inconsistencies on Windows. Will rebase this after that ends up in main
--
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: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] GH-46797: [C++] Support Meson on Windows [arrow]
Posted by "WillAyd (via GitHub)" <gi...@apache.org>.
WillAyd commented on code in PR #47282:
URL: https://github.com/apache/arrow/pull/47282#discussion_r2335177775
##########
cpp/src/arrow/meson.build:
##########
@@ -420,19 +481,38 @@ foreach key, val : arrow_components
arrow_deps += val.get('dependencies', [])
endforeach
+arrow_lib_cpp_shared_args = ['-DARROW_EXPORTING', '-DURI_LIBRARY_BUILD']
+if needs_compute
+ arrow_lib_cpp_shared_args += ['-DARROW_COMPUTE_EXPORTING']
+endif
+
arrow_lib = library(
'arrow',
sources: arrow_srcs,
include_directories: arrow_includes,
dependencies: arrow_deps,
install: true,
gnu_symbol_visibility: 'inlineshidden',
- cpp_shared_args: ['-DARROW_EXPORTING'],
+ gnu_symbol_visibility: 'hidden',
+ c_shared_args: [
+ '-DARROW_EXPORTING',
+ '-DURI_LIBRARY_BUILD',
+ '-DURI_VISIBILITY',
Review Comment:
It does not - that macro is used by the vendored/uriparser code which is built via the C compiler
--
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: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] GH-46797: [C++] Support Meson on Windows [arrow]
Posted by "kou (via GitHub)" <gi...@apache.org>.
kou commented on code in PR #47282:
URL: https://github.com/apache/arrow/pull/47282#discussion_r2292475948
##########
.github/workflows/cpp_extra.yml:
##########
@@ -278,3 +279,70 @@ jobs:
cmake --build cpp/examples/minimal_build.build
cd cpp/examples/minimal_build
../minimal_build.build/arrow-example
+
+ meson-windows:
+ needs: check-labels
+ name: ${{ matrix.title }}
+ runs-on: ${{ matrix.runs-on }}
+ if: needs.check-labels.outputs.ci-extra == 'true'
+ timeout-minutes: 75
+ strategy:
+ fail-fast: false
+ matrix:
+ include:
+ - runs-on: windows-2022
+ title: AMD64 Windows Meson
+ steps:
+ - name: Checkout Arrow
+ uses: actions/checkout@v4
+ with:
+ fetch-depth: 0
+ submodules: recursive
+ - name: Download Timezone Database
+ shell: bash
+ run: ci/scripts/download_tz_database.sh
+ - 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: cpp-ccache-windows-meson-${{ env.CACHE_VERSION }}-${{ hashFiles('cpp/**') }}
+ restore-keys: cpp-ccache-windows-meson-${{ env.CACHE_VERSION }}-
+ env:
+ # We can invalidate the current cache by updating this.
+ CACHE_VERSION: "2022-09-13"
Review Comment:
OK!
--
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: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] GH-46797: [C++] Support Meson on Windows [arrow]
Posted by "WillAyd (via GitHub)" <gi...@apache.org>.
WillAyd commented on code in PR #47282:
URL: https://github.com/apache/arrow/pull/47282#discussion_r2291842801
##########
cpp/src/arrow/flight/meson.build:
##########
@@ -44,7 +44,17 @@ install_headers(
subdir: 'arrow/flight',
)
-grpc_dep = dependency('grpc++')
+# We specifically need to use the grpc subproject and cannot use a system installion
+# due to https://github.com/mesonbuild/meson/issues/14905
Review Comment:
Actually I think this current iteration will have the same issue of https://github.com/mesonbuild/meson/issues/14905 when there is a system install of grpc. Happy to leave as is though as it won't affect CI.
--
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: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] GH-46797: [C++] Support Meson on Windows [arrow]
Posted by "kou (via GitHub)" <gi...@apache.org>.
kou commented on PR #47282:
URL: https://github.com/apache/arrow/pull/47282#issuecomment-3294493558
> somewhat relatedly is it intentional that the Windows CI not use conda? I used an existing job as a template; it would simplify things and help with build times I think though if we used conda and have pre-installed third party dependencies
There is no strong reason. But the existing Windows CI is suitable to check bundled dependencies.
We can use conda for Meson CI job.
--
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: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] GH-46797: [C++] Support Meson on Windows [arrow]
Posted by "WillAyd (via GitHub)" <gi...@apache.org>.
WillAyd commented on PR #47282:
URL: https://github.com/apache/arrow/pull/47282#issuecomment-3186774629
Hmm it looks like that failing test is going to require some of the boost libraries that enable threading, which aren't buildable via the Meson CMake wrapper on Windows. For now we might just need to disable that one test (perhaps it should be tied to the definition of `ARROW_ENABLE_THREADING`?)
--
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: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] GH-46797: [C++] Support Meson on Windows [arrow]
Posted by "github-actions[bot] (via GitHub)" <gi...@apache.org>.
github-actions[bot] commented on PR #47282:
URL: https://github.com/apache/arrow/pull/47282#issuecomment-3167924920
:warning: GitHub issue #46797 **has been automatically assigned in GitHub** to PR creator.
--
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: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] GH-46797: [C++] Support Meson on Windows [arrow]
Posted by "WillAyd (via GitHub)" <gi...@apache.org>.
WillAyd commented on code in PR #47282:
URL: https://github.com/apache/arrow/pull/47282#discussion_r2263046241
##########
cpp/src/arrow/meson.build:
##########
@@ -423,12 +435,21 @@ arrow_lib = library(
dependencies: arrow_deps,
install: true,
gnu_symbol_visibility: 'hidden',
- cpp_shared_args: ['-DARROW_EXPORTING'],
+ c_shared_args: ['-DARROW_EXPORTING', '-DURI_LIBRARY_BUILD', '-DURI_VISIBILITY'],
+ c_static_args: ['-DURI_STATIC_BUILD'],
+ cpp_shared_args: ['-DARROW_EXPORTING', '-DURI_LIBRARY_BUILD'],
+ cpp_static_args: ['-DARROW_STATIC'],
)
+arrow_compile_args = []
+if host_machine.system() == 'windows' and get_option('default_library') != 'shared'
Review Comment:
I haven't gotten `default_library=both` to work yet, but haven't put a ton of effort into debugging it either. Happy to do so after getting shared/static support stabilized
--
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: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] GH-46797: [C++] Support Meson on Windows [arrow]
Posted by "kou (via GitHub)" <gi...@apache.org>.
kou commented on PR #47282:
URL: https://github.com/apache/arrow/pull/47282#issuecomment-3203965859
Correct: #46690
--
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: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] GH-46797: [C++] Support Meson on Windows [arrow]
Posted by "kou (via GitHub)" <gi...@apache.org>.
kou commented on code in PR #47282:
URL: https://github.com/apache/arrow/pull/47282#discussion_r2335216822
##########
cpp/src/arrow/meson.build:
##########
@@ -420,19 +481,38 @@ foreach key, val : arrow_components
arrow_deps += val.get('dependencies', [])
endforeach
+arrow_lib_cpp_shared_args = ['-DARROW_EXPORTING', '-DURI_LIBRARY_BUILD']
+if needs_compute
+ arrow_lib_cpp_shared_args += ['-DARROW_COMPUTE_EXPORTING']
+endif
+
arrow_lib = library(
'arrow',
sources: arrow_srcs,
include_directories: arrow_includes,
dependencies: arrow_deps,
install: true,
gnu_symbol_visibility: 'inlineshidden',
- cpp_shared_args: ['-DARROW_EXPORTING'],
+ gnu_symbol_visibility: 'hidden',
+ c_shared_args: [
+ '-DARROW_EXPORTING',
+ '-DURI_LIBRARY_BUILD',
+ '-DURI_VISIBILITY',
Review Comment:
Hmm. It seems that this C++ source https://github.com/apache/arrow/blob/417df286dfaf0a20280684fd0093e5fac589e42b/cpp/src/arrow/util/uri.cc#L28 also uses it.
BTW, why do we need to use `-DURI_LIBRARY_BUILD`? CMake uses `URI_STATIC_BUILD` because vendored/uriparser is built into `libarrow`: https://github.com/apache/arrow/blob/417df286dfaf0a20280684fd0093e5fac589e42b/cpp/src/arrow/CMakeLists.txt#L478
--
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: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] GH-46797: [C++] Support Meson on Windows [arrow]
Posted by "kou (via GitHub)" <gi...@apache.org>.
kou commented on code in PR #47282:
URL: https://github.com/apache/arrow/pull/47282#discussion_r2338305254
##########
cpp/src/arrow/meson.build:
##########
@@ -420,19 +481,38 @@ foreach key, val : arrow_components
arrow_deps += val.get('dependencies', [])
endforeach
+arrow_lib_cpp_shared_args = ['-DARROW_EXPORTING', '-DURI_LIBRARY_BUILD']
+if needs_compute
+ arrow_lib_cpp_shared_args += ['-DARROW_COMPUTE_EXPORTING']
+endif
+
arrow_lib = library(
'arrow',
sources: arrow_srcs,
include_directories: arrow_includes,
dependencies: arrow_deps,
install: true,
gnu_symbol_visibility: 'inlineshidden',
- cpp_shared_args: ['-DARROW_EXPORTING'],
+ gnu_symbol_visibility: 'hidden',
+ c_shared_args: [
+ '-DARROW_EXPORTING',
+ '-DURI_LIBRARY_BUILD',
+ '-DURI_VISIBILITY',
Review Comment:
We need `-DURI_STATIC_BUILD` for https://github.com/apache/arrow/blob/37faf3e79512c2c036b004d80770bb522af035f1/cpp/src/arrow/meson.build#L99-L128 too.
--
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: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] GH-46797: [C++] Support Meson on Windows [arrow]
Posted by "WillAyd (via GitHub)" <gi...@apache.org>.
WillAyd commented on PR #47282:
URL: https://github.com/apache/arrow/pull/47282#issuecomment-3292720843
Looks like building thrift as a subproject on Windows is failing to locate the Boost dependency. Will take a closer look.
@kou somewhat relatedly is it intentional that the Windows CI not use conda? I used an existing job as a template; it would simplify things and help with build times I think though if we used conda and have pre-installed third party dependencies
--
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: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] GH-46797: [C++] Support Meson on Windows [arrow]
Posted by "WillAyd (via GitHub)" <gi...@apache.org>.
WillAyd commented on code in PR #47282:
URL: https://github.com/apache/arrow/pull/47282#discussion_r2360199711
##########
cpp/src/parquet/meson.build:
##########
@@ -57,7 +57,12 @@ parquet_srcs = files(
'xxhasher.cc',
)
-thrift_dep = dependency('thrift', allow_fallback: false, required: false)
+thrift_dep = dependency(
+ 'thrift',
+ allow_fallback: false,
+ method: host_machine.system() == 'windows' ? 'cmake' : 'auto',
Review Comment:
This appears to be an upstream issue where the library names used in the pkg-config files do not match the library names distributed on windows. See https://issues.apache.org/jira/browse/THRIFT-5894
--
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: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] GH-46797: [C++] Support Meson on Windows [arrow]
Posted by "WillAyd (via GitHub)" <gi...@apache.org>.
WillAyd commented on PR #47282:
URL: https://github.com/apache/arrow/pull/47282#issuecomment-3436906796
I am still looking at this, but I think it will take a bit longer unfortunately. gRPC has some custom library naming conventions on Windows that don't appear reflected in the upstream Meson wrap entry. Its not possible to use the conda packaged gRPC libraries because they have symbol visibility issues on Windows, so the former will likely need to be fixed before this can continue
--
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: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] GH-46797: [C++] Support Meson on Windows [arrow]
Posted by "WillAyd (via GitHub)" <gi...@apache.org>.
WillAyd commented on code in PR #47282:
URL: https://github.com/apache/arrow/pull/47282#discussion_r2333489575
##########
cpp/src/arrow/meson.build:
##########
@@ -420,19 +481,38 @@ foreach key, val : arrow_components
arrow_deps += val.get('dependencies', [])
endforeach
+arrow_lib_cpp_shared_args = ['-DARROW_EXPORTING', '-DURI_LIBRARY_BUILD']
+if needs_compute
+ arrow_lib_cpp_shared_args += ['-DARROW_COMPUTE_EXPORTING']
+endif
+
arrow_lib = library(
'arrow',
sources: arrow_srcs,
include_directories: arrow_includes,
dependencies: arrow_deps,
install: true,
gnu_symbol_visibility: 'inlineshidden',
- cpp_shared_args: ['-DARROW_EXPORTING'],
+ gnu_symbol_visibility: 'hidden',
+ c_shared_args: [
+ '-DARROW_EXPORTING',
+ '-DURI_LIBRARY_BUILD',
+ '-DURI_VISIBILITY',
Review Comment:
The C++ defines are managed through the `arrow_lib_cpp_shared_args` variable, whose contents may differ depending on whether or not the compute option is enabled
--
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: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] GH-46797: [C++] Support Meson on Windows [arrow]
Posted by "WillAyd (via GitHub)" <gi...@apache.org>.
WillAyd commented on PR #47282:
URL: https://github.com/apache/arrow/pull/47282#issuecomment-3186678973
Ah I see. Ok so the issue is that Boost.Process is required for the test - will see what we need to do to make that available. Thanks!
--
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: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] GH-46797: [C++] Support Meson on Windows [arrow]
Posted by "WillAyd (via GitHub)" <gi...@apache.org>.
WillAyd commented on PR #47282:
URL: https://github.com/apache/arrow/pull/47282#issuecomment-3184277173
Here's the most recent run with the same issue:
https://github.com/apache/arrow/actions/runs/16939693514/job/48005185405?pr=47282#step:11:220
--
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: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] GH-46797: [C++] Support Meson on Windows [arrow]
Posted by "WillAyd (via GitHub)" <gi...@apache.org>.
WillAyd commented on code in PR #47282:
URL: https://github.com/apache/arrow/pull/47282#discussion_r2270999922
##########
cpp/src/arrow/testing/gtest_util_test.cc:
##########
@@ -287,9 +287,11 @@ TEST(TestWithinUlp, Float) {
TEST(AssertTestWithinUlp, Basics) {
AssertWithinUlp(123.4567, 123.45670000000015, 11);
AssertWithinUlp(123.456f, 123.456085f, 11);
+#if !defined(_MSC_VER)
Review Comment:
I'm not sure why the precision here is different for Windows - open to any ideas
##########
.github/workflows/cpp_extra.yml:
##########
@@ -174,3 +174,70 @@ jobs:
ARCHERY_DOCKER_PASSWORD: ${{ secrets.DOCKERHUB_TOKEN }}
continue-on-error: true
run: archery docker push ${{ matrix.image }}
+
+ meson-windows:
+ needs: check-labels
+ name: ${{ matrix.title }}
+ runs-on: ${{ matrix.runs-on }}
+ if: needs.check-labels.outputs.ci-extra == 'true'
+ timeout-minutes: 75
+ strategy:
+ fail-fast: false
+ matrix:
+ include:
+ - runs-on: windows-2022
+ title: AMD64 Windows Meson
+ steps:
+ - name: Checkout Arrow
+ uses: actions/checkout@v4
+ with:
+ fetch-depth: 0
+ submodules: recursive
+ - name: Download Timezone Database
+ shell: bash
+ run: ci/scripts/download_tz_database.sh
+ - 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: cpp-ccache-windows-meson-${{ env.CACHE_VERSION }}-${{ hashFiles('cpp/**') }}
+ restore-keys: cpp-ccache-windows-meson-${{ env.CACHE_VERSION }}-
+ env:
+ # We can invalidate the current cache by updating this.
+ CACHE_VERSION: "2022-09-13"
+ - name: Setup Python
+ uses: actions/setup-python@a26af69be951a213d495a4c3e4e4022e16d87065 # v5.6.0
+ with:
+ python-version: 3
+ - name: Install Meson
+ run: |
+ python3 -m pip install meson
+ - name: Build
+ shell: cmd
+ env:
+ ARROW_USE_MESON: ON
+ ARROW_HOME: C:\arrow-dist
+ run: |
+ meson setup cpp build/cpp --vsenv -Dauto_features=enabled -Dfuzzing=disabled -Dgcs=disabled -Ds3=disabled -Dazure=disabled
Review Comment:
Because the Windows version of the meson setup has slightly different arguments (adds --vsenv and disables azure) I just invoked Meson directly here.
If we want to add to the existing cpp_build.sh script I can look at passing via an environment variable or script argument, although maybe its just as easy to invoke meson directly?
--
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: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] GH-46797: [C++] Support Meson on Windows [arrow]
Posted by "WillAyd (via GitHub)" <gi...@apache.org>.
WillAyd commented on code in PR #47282:
URL: https://github.com/apache/arrow/pull/47282#discussion_r2270999922
##########
cpp/src/arrow/testing/gtest_util_test.cc:
##########
@@ -287,9 +287,11 @@ TEST(TestWithinUlp, Float) {
TEST(AssertTestWithinUlp, Basics) {
AssertWithinUlp(123.4567, 123.45670000000015, 11);
AssertWithinUlp(123.456f, 123.456085f, 11);
+#if !defined(_MSC_VER)
Review Comment:
I'm not sure why the precision here is different for MSVC- open to any ideas
--
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: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] GH-46797: [C++] Support Meson on Windows [arrow]
Posted by "WillAyd (via GitHub)" <gi...@apache.org>.
WillAyd commented on PR #47282:
URL: https://github.com/apache/arrow/pull/47282#issuecomment-3208073783
@kou do you have any thoughts on the content of the PR at this stage?
--
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: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] GH-46797: [C++] Support Meson on Windows [arrow]
Posted by "kou (via GitHub)" <gi...@apache.org>.
kou commented on code in PR #47282:
URL: https://github.com/apache/arrow/pull/47282#discussion_r2335141617
##########
cpp/src/arrow/meson.build:
##########
@@ -420,19 +481,38 @@ foreach key, val : arrow_components
arrow_deps += val.get('dependencies', [])
endforeach
+arrow_lib_cpp_shared_args = ['-DARROW_EXPORTING', '-DURI_LIBRARY_BUILD']
+if needs_compute
+ arrow_lib_cpp_shared_args += ['-DARROW_COMPUTE_EXPORTING']
+endif
+
arrow_lib = library(
'arrow',
sources: arrow_srcs,
include_directories: arrow_includes,
dependencies: arrow_deps,
install: true,
gnu_symbol_visibility: 'inlineshidden',
- cpp_shared_args: ['-DARROW_EXPORTING'],
+ gnu_symbol_visibility: 'hidden',
+ c_shared_args: [
+ '-DARROW_EXPORTING',
+ '-DURI_LIBRARY_BUILD',
+ '-DURI_VISIBILITY',
Review Comment:
Ah, I want to confirm whether `arrow_lib_cpp_shared_args` needs `-DURI_VISIBILITY` or not.
##########
cpp/src/arrow/flight/meson.build:
##########
@@ -44,7 +44,25 @@ install_headers(
subdir: 'arrow/flight',
)
-grpc_dep = dependency('grpc++')
+grpc_opts = []
+if cpp_compiler.get_id() == 'msvc'
+ grpc_opts += [
+ 'openssl:asm=disabled', # asm cannot compile on Windows
+ 're2:default_library=static', # re2 does not export symbols
+ ]
+endif
+
+# Due to the Meson bug in https://github.com/mesonbuild/meson/issues/14905
+# we need to special how the grpc_cpp_plugin resolution is handled
+# when found on the system versus when built as a subproject
+grpc_cpp_dep = dependency('grpc++', allow_fallback: false, required: false)
+if grpc_cpp_dep.found()
+ has_system_grpc = true
+else
+ grpc_cpp_dep = dependency('grpc++')
+ has_system_grpc = false
+endif
Review Comment:
Can we simplify this?
```suggestion
has_system_grpc = grpc_cpp_dep.found()
if not has_system_grpc
grpc_cpp_dep = dependency('grpc++')
endif
```
--
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: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] GH-46797: [C++] Support Meson on Windows [arrow]
Posted by "WillAyd (via GitHub)" <gi...@apache.org>.
WillAyd commented on code in PR #47282:
URL: https://github.com/apache/arrow/pull/47282#discussion_r2291456685
##########
.github/workflows/cpp_extra.yml:
##########
@@ -278,3 +279,70 @@ jobs:
cmake --build cpp/examples/minimal_build.build
cd cpp/examples/minimal_build
../minimal_build.build/arrow-example
+
+ meson-windows:
+ needs: check-labels
+ name: ${{ matrix.title }}
+ runs-on: ${{ matrix.runs-on }}
+ if: needs.check-labels.outputs.ci-extra == 'true'
+ timeout-minutes: 75
+ strategy:
+ fail-fast: false
+ matrix:
+ include:
+ - runs-on: windows-2022
+ title: AMD64 Windows Meson
+ steps:
+ - name: Checkout Arrow
+ uses: actions/checkout@v4
+ with:
+ fetch-depth: 0
+ submodules: recursive
+ - name: Download Timezone Database
+ shell: bash
+ run: ci/scripts/download_tz_database.sh
+ - 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: cpp-ccache-windows-meson-${{ env.CACHE_VERSION }}-${{ hashFiles('cpp/**') }}
+ restore-keys: cpp-ccache-windows-meson-${{ env.CACHE_VERSION }}-
+ env:
+ # We can invalidate the current cache by updating this.
+ CACHE_VERSION: "2022-09-13"
Review Comment:
I'm not sure what date this is supposed to be so just picked today's. Hope that is OK!
##########
cpp/src/arrow/compute/CMakeLists.txt:
##########
@@ -52,6 +52,7 @@ if(ARROW_TESTING AND ARROW_COMPUTE)
target_link_libraries(arrow_compute_testing
PUBLIC $<TARGET_OBJECTS:arrow_compute_core_testing>
PUBLIC ${ARROW_GTEST_GTEST_MAIN})
+ target_compile_definitions(arrow_compute_testing PRIVATE COMPILED_BY_CMAKE)
Review Comment:
While there is a gtest version difference, I don't think that is the problem. The CMake configuration has had a few issues dating back to https://github.com/apache/arrow/pull/46839 and https://github.com/apache/arrow/pull/47049 and it currently special cases the gtest dependency linkage (the Meson configuration does not, but also seems OK with it...)
##########
cpp/src/arrow/flight/meson.build:
##########
@@ -44,7 +44,17 @@ install_headers(
subdir: 'arrow/flight',
)
-grpc_dep = dependency('grpc++')
+# We specifically need to use the grpc subproject and cannot use a system installion
+# due to https://github.com/mesonbuild/meson/issues/14905
Review Comment:
Using full_path like that is unfortunately not an option and will yield errors like:
```
ninja: error: 'grpc_cpp_plugin.exe', needed by 'src/arrow/flight/Flight.grpc.pb.h', missing and no known rule to make it
```
In any case the pkg-config name was always wrong. I've updated that here - maybe that was the problem?
--
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: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] GH-46797: [C++] Support Meson on Windows [arrow]
Posted by "kou (via GitHub)" <gi...@apache.org>.
kou commented on code in PR #47282:
URL: https://github.com/apache/arrow/pull/47282#discussion_r2264564298
##########
cpp/src/arrow/meson.build:
##########
@@ -484,14 +505,22 @@ if needs_compute
arrow_compute_lib = library(
'arrow-compute',
sources: arrow_compute_lib_sources,
- dependencies: arrow_dep,
+ dependencies: [arrow_dep] + int128_deps,
Review Comment:
Ah, compute also uses `arrow::internal::int128_t`. Sorry. I thought that decimal in libarrow only uses `arrow::internal::int128_t`.
--
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: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] GH-46797: [C++] Support Meson on Windows [arrow]
Posted by "WillAyd (via GitHub)" <gi...@apache.org>.
WillAyd commented on PR #47282:
URL: https://github.com/apache/arrow/pull/47282#issuecomment-3184025299
> @WillAyd where in the logs is this?
Here's one of the older CIs showing it:
https://github.com/apache/arrow/actions/runs/16915886894/job/47929561368#step:11:399
--
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: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] GH-46797: [C++] Support Meson on Windows [arrow]
Posted by "WillAyd (via GitHub)" <gi...@apache.org>.
WillAyd commented on code in PR #47282:
URL: https://github.com/apache/arrow/pull/47282#discussion_r2301885523
##########
cpp/src/arrow/testing/gtest_util_test.cc:
##########
@@ -287,9 +287,11 @@ TEST(TestWithinUlp, Float) {
TEST(AssertTestWithinUlp, Basics) {
AssertWithinUlp(123.4567, 123.45670000000015, 11);
AssertWithinUlp(123.456f, 123.456085f, 11);
+#if !defined(_MSC_VER)
Review Comment:
So upon investigating this some more, this actually _does_ fail with the CMake build. However, there seems to be something interesting going on where many of the CMake tests are not actually being run on Windows.
For instance, with a fresh project setup, if I do:
```sh
$ cmake -S . -B build -DARROW_COMPUTE=ON -DARROW_BUILD_TESTS=ON
$ cd build
$ ctest -R arrow-gtest-util-test --verbose
```
Everything comes back passing, but no tests are actually being run.
If I add a custom main function to `gtest_util_test.cc`:
```cpp
int main(int argc, char **argv) {
::testing::InitGoogleTest(&argc, argv);
return RUN_ALL_TESTS();
}
```
recompile, and then re-run the ctest command, I get the same failure shown here:
```sh
PS C:\Users\Alison\clones\arrow\cpp\build> ctest -R gtest-util-test --verbose
UpdateCTestConfiguration from :C:/Users/Alison/clones/arrow/cpp/build/DartConfiguration.tcl
UpdateCTestConfiguration from :C:/Users/Alison/clones/arrow/cpp/build/DartConfiguration.tcl
Test project C:/Users/Alison/clones/arrow/cpp/build
Constructing a list of tests
Done constructing a list of tests
Updating test list for fixtures
Added 0 tests to meet fixture requirements
Checking test dependency graph...
Checking test dependency graph end
test 13
Start 13: arrow-gtest-util-test
13: Test command: C:\Users\Alison\clones\arrow\cpp\build\debug\arrow-gtest-util-test.exe
13: Working Directory: C:/Users/Alison/clones/arrow/cpp/build/src/arrow/testing
13: Test timeout computed to be: 10000000
13: [==========] Running 10 tests from 4 test suites.
13: [----------] Global test environment set-up.
13: [----------] 5 tests from TestAssertContainsNaN
13: [ RUN ] TestAssertContainsNaN.BatchesEqual
13: [ OK ] TestAssertContainsNaN.BatchesEqual (1 ms)
13: [ RUN ] TestAssertContainsNaN.TableEqual
13: [ OK ] TestAssertContainsNaN.TableEqual (0 ms)
13: [ RUN ] TestAssertContainsNaN.ArrayEqual
13: [ OK ] TestAssertContainsNaN.ArrayEqual (0 ms)
13: [ RUN ] TestAssertContainsNaN.ChunkedEqual
13: [ OK ] TestAssertContainsNaN.ChunkedEqual (0 ms)
13: [ RUN ] TestAssertContainsNaN.DatumEqual
13: [ OK ] TestAssertContainsNaN.DatumEqual (0 ms)
13: [----------] 5 tests from TestAssertContainsNaN (2 ms total)
13:
13: [----------] 2 tests from TestTensorFromJSON
13: [ RUN ] TestTensorFromJSON.FromJSONAndArray
13: [ OK ] TestTensorFromJSON.FromJSONAndArray (0 ms)
13: [ RUN ] TestTensorFromJSON.FromJSON
13: [ OK ] TestTensorFromJSON.FromJSON (0 ms)
13: [----------] 2 tests from TestTensorFromJSON (0 ms total)
13:
13: [----------] 2 tests from TestWithinUlp
13: [ RUN ] TestWithinUlp.Double
13: [ OK ] TestWithinUlp.Double (3 ms)
13: [ RUN ] TestWithinUlp.Float
13: [ OK ] TestWithinUlp.Float (3 ms)
13: [----------] 2 tests from TestWithinUlp (7 ms total)
13:
13: [----------] 1 test from AssertTestWithinUlp
13: [ RUN ] AssertTestWithinUlp.Basics
13: C:\Users\Alison\clones\arrow\cpp\src\arrow\testing\math.cc(72): error: Failed
13: 123.4567 and 123.45670000000015 are not within 10 ulps
13:
13: C:\Users\Alison\clones\arrow\cpp\build\_deps\googletest-src\googletest\src/gtest.cc(1046): error: Expected: 1 fatal failure
13: Actual: 0 failures
13:
13: C:\Users\Alison\clones\arrow\cpp\src\arrow\testing\math.cc(72): error: Failed
13: 123.45600128173828 and 123.45608520507812 are not within 10 ulps
13:
13: C:\Users\Alison\clones\arrow\cpp\build\_deps\googletest-src\googletest\src/gtest.cc(1046): error: Expected: 1 fatal failure
13: Actual: 0 failures
13:
13: [ FAILED ] AssertTestWithinUlp.Basics (0 ms)
13: [----------] 1 test from AssertTestWithinUlp (0 ms total)
13:
13: [----------] Global test environment tear-down
13: [==========] 10 tests from 4 test suites ran. (11 ms total)
13: [ PASSED ] 9 tests.
13: [ FAILED ] 1 test, listed below:
13: [ FAILED ] AssertTestWithinUlp.Basics
13:
13: 1 FAILED TEST
1/1 Test #13: arrow-gtest-util-test ............***Failed 0.15 sec
0% tests passed, 1 tests failed out of 1
Label Time Summary:
arrow-tests = 0.15 sec*proc (1 test)
unittest = 0.15 sec*proc (1 test)
Total Test time (real) = 0.17 sec
The following tests FAILED:
13 - arrow-gtest-util-test (Failed) arrow-tests unittest
```
--
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: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] GH-46797: [C++] Support Meson on Windows [arrow]
Posted by "WillAyd (via GitHub)" <gi...@apache.org>.
WillAyd commented on code in PR #47282:
URL: https://github.com/apache/arrow/pull/47282#discussion_r2301912341
##########
cpp/src/arrow/testing/gtest_util_test.cc:
##########
@@ -287,9 +287,11 @@ TEST(TestWithinUlp, Float) {
TEST(AssertTestWithinUlp, Basics) {
AssertWithinUlp(123.4567, 123.45670000000015, 11);
AssertWithinUlp(123.456f, 123.456085f, 11);
+#if !defined(_MSC_VER)
Review Comment:
I opened https://github.com/apache/arrow/issues/47434 for this as I think it is a larger issue
--
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: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] GH-46797: [C++] Support Meson on Windows [arrow]
Posted by "WillAyd (via GitHub)" <gi...@apache.org>.
WillAyd commented on PR #47282:
URL: https://github.com/apache/arrow/pull/47282#issuecomment-3330394530
Still looking at this - unfortunately it looks like Thrift must be located on the system (hence the conda usage), but the grpc/libprotobuf used in conda don't seem to play nicely with the symbol visibility expectations on Windows.
--
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: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] GH-46797: [C++] Support Meson on Windows [arrow]
Posted by "WillAyd (via GitHub)" <gi...@apache.org>.
WillAyd commented on code in PR #47282:
URL: https://github.com/apache/arrow/pull/47282#discussion_r2301885523
##########
cpp/src/arrow/testing/gtest_util_test.cc:
##########
@@ -287,9 +287,11 @@ TEST(TestWithinUlp, Float) {
TEST(AssertTestWithinUlp, Basics) {
AssertWithinUlp(123.4567, 123.45670000000015, 11);
AssertWithinUlp(123.456f, 123.456085f, 11);
+#if !defined(_MSC_VER)
Review Comment:
So upon investigating this some more, this actually _does_ fail with the CMake build. However, there seems to be something interesting going on where many of the CMake tests are not actually being run on Windows.
For instance, with a fresh project setup, if I do:
```sh
$ cmake -S . -B build -G Ninja -DARROW_COMPUTE=ON -DARROW_BUILD_TESTS=ON
$ cd build
$ ctest -R arrow-gtest-util-test --verbose
```
Everything comes back passing, but no tests are actually being run.
If I add a custom main function to `gtest_util_test.cc`:
```cpp
int main(int argc, char **argv) {
::testing::InitGoogleTest(&argc, argv);
return RUN_ALL_TESTS();
}
```
recompile, and then re-run the ctest command, I get the same failure shown here:
```sh
PS C:\Users\Alison\clones\arrow\cpp\build> ctest -R gtest-util-test --verbose
UpdateCTestConfiguration from :C:/Users/Alison/clones/arrow/cpp/build/DartConfiguration.tcl
UpdateCTestConfiguration from :C:/Users/Alison/clones/arrow/cpp/build/DartConfiguration.tcl
Test project C:/Users/Alison/clones/arrow/cpp/build
Constructing a list of tests
Done constructing a list of tests
Updating test list for fixtures
Added 0 tests to meet fixture requirements
Checking test dependency graph...
Checking test dependency graph end
test 13
Start 13: arrow-gtest-util-test
13: Test command: C:\Users\Alison\clones\arrow\cpp\build\debug\arrow-gtest-util-test.exe
13: Working Directory: C:/Users/Alison/clones/arrow/cpp/build/src/arrow/testing
13: Test timeout computed to be: 10000000
13: [==========] Running 10 tests from 4 test suites.
13: [----------] Global test environment set-up.
13: [----------] 5 tests from TestAssertContainsNaN
13: [ RUN ] TestAssertContainsNaN.BatchesEqual
13: [ OK ] TestAssertContainsNaN.BatchesEqual (1 ms)
13: [ RUN ] TestAssertContainsNaN.TableEqual
13: [ OK ] TestAssertContainsNaN.TableEqual (0 ms)
13: [ RUN ] TestAssertContainsNaN.ArrayEqual
13: [ OK ] TestAssertContainsNaN.ArrayEqual (0 ms)
13: [ RUN ] TestAssertContainsNaN.ChunkedEqual
13: [ OK ] TestAssertContainsNaN.ChunkedEqual (0 ms)
13: [ RUN ] TestAssertContainsNaN.DatumEqual
13: [ OK ] TestAssertContainsNaN.DatumEqual (0 ms)
13: [----------] 5 tests from TestAssertContainsNaN (2 ms total)
13:
13: [----------] 2 tests from TestTensorFromJSON
13: [ RUN ] TestTensorFromJSON.FromJSONAndArray
13: [ OK ] TestTensorFromJSON.FromJSONAndArray (0 ms)
13: [ RUN ] TestTensorFromJSON.FromJSON
13: [ OK ] TestTensorFromJSON.FromJSON (0 ms)
13: [----------] 2 tests from TestTensorFromJSON (0 ms total)
13:
13: [----------] 2 tests from TestWithinUlp
13: [ RUN ] TestWithinUlp.Double
13: [ OK ] TestWithinUlp.Double (3 ms)
13: [ RUN ] TestWithinUlp.Float
13: [ OK ] TestWithinUlp.Float (3 ms)
13: [----------] 2 tests from TestWithinUlp (7 ms total)
13:
13: [----------] 1 test from AssertTestWithinUlp
13: [ RUN ] AssertTestWithinUlp.Basics
13: C:\Users\Alison\clones\arrow\cpp\src\arrow\testing\math.cc(72): error: Failed
13: 123.4567 and 123.45670000000015 are not within 10 ulps
13:
13: C:\Users\Alison\clones\arrow\cpp\build\_deps\googletest-src\googletest\src/gtest.cc(1046): error: Expected: 1 fatal failure
13: Actual: 0 failures
13:
13: C:\Users\Alison\clones\arrow\cpp\src\arrow\testing\math.cc(72): error: Failed
13: 123.45600128173828 and 123.45608520507812 are not within 10 ulps
13:
13: C:\Users\Alison\clones\arrow\cpp\build\_deps\googletest-src\googletest\src/gtest.cc(1046): error: Expected: 1 fatal failure
13: Actual: 0 failures
13:
13: [ FAILED ] AssertTestWithinUlp.Basics (0 ms)
13: [----------] 1 test from AssertTestWithinUlp (0 ms total)
13:
13: [----------] Global test environment tear-down
13: [==========] 10 tests from 4 test suites ran. (11 ms total)
13: [ PASSED ] 9 tests.
13: [ FAILED ] 1 test, listed below:
13: [ FAILED ] AssertTestWithinUlp.Basics
13:
13: 1 FAILED TEST
1/1 Test #13: arrow-gtest-util-test ............***Failed 0.15 sec
0% tests passed, 1 tests failed out of 1
Label Time Summary:
arrow-tests = 0.15 sec*proc (1 test)
unittest = 0.15 sec*proc (1 test)
Total Test time (real) = 0.17 sec
The following tests FAILED:
13 - arrow-gtest-util-test (Failed) arrow-tests unittest
```
--
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: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] GH-46797: [C++] Support Meson on Windows [arrow]
Posted by "WillAyd (via GitHub)" <gi...@apache.org>.
WillAyd commented on PR #47282:
URL: https://github.com/apache/arrow/pull/47282#issuecomment-3228680285
I think the remaining issues are related to CMake and how it chooses to link the various test libraries, and what effects that has on the execution of them. I've opened the testing issue accordingly to take a look at that outside of this PR.
Current CI failures I believe are also unrelated
--
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: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] GH-46797: [C++] Support Meson on Windows [arrow]
Posted by "kou (via GitHub)" <gi...@apache.org>.
kou commented on PR #47282:
URL: https://github.com/apache/arrow/pull/47282#issuecomment-3283102261
Could you rebase on main?
--
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: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] GH-46797: [C++] Support Meson on Windows [arrow]
Posted by "kou (via GitHub)" <gi...@apache.org>.
kou commented on code in PR #47282:
URL: https://github.com/apache/arrow/pull/47282#discussion_r2331750064
##########
.github/workflows/cpp_extra.yml:
##########
@@ -278,3 +279,70 @@ jobs:
cmake --build cpp/examples/minimal_build.build
cd cpp/examples/minimal_build
../minimal_build.build/arrow-example
+
+ meson-windows:
+ needs: check-labels
+ name: ${{ matrix.title }}
+ runs-on: ${{ matrix.runs-on }}
+ if: needs.check-labels.outputs.ci-extra == 'true'
+ timeout-minutes: 75
+ strategy:
+ fail-fast: false
+ matrix:
+ include:
+ - runs-on: windows-2022
+ title: AMD64 Windows Meson
+ steps:
+ - name: Checkout Arrow
+ uses: actions/checkout@v4
+ with:
+ fetch-depth: 0
+ submodules: recursive
+ - name: Download Timezone Database
+ shell: bash
+ run: ci/scripts/download_tz_database.sh
+ - 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
Review Comment:
```suggestion
uses: actions/cache@5a3ec84eff668545956fd18022155c47e93e2684 # v4.2.3
```
##########
.github/workflows/cpp_extra.yml:
##########
@@ -278,3 +279,70 @@ jobs:
cmake --build cpp/examples/minimal_build.build
cd cpp/examples/minimal_build
../minimal_build.build/arrow-example
+
+ meson-windows:
+ needs: check-labels
+ name: ${{ matrix.title }}
+ runs-on: ${{ matrix.runs-on }}
+ if: needs.check-labels.outputs.ci-extra == 'true'
+ timeout-minutes: 75
+ strategy:
+ fail-fast: false
+ matrix:
+ include:
+ - runs-on: windows-2022
+ title: AMD64 Windows Meson
+ steps:
+ - name: Checkout Arrow
+ uses: actions/checkout@v4
+ with:
+ fetch-depth: 0
+ submodules: recursive
+ - name: Download Timezone Database
+ shell: bash
+ run: ci/scripts/download_tz_database.sh
+ - 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: cpp-ccache-windows-meson-${{ env.CACHE_VERSION }}-${{ hashFiles('cpp/**') }}
+ restore-keys: cpp-ccache-windows-meson-${{ env.CACHE_VERSION }}-
+ env:
+ # We can invalidate the current cache by updating this.
+ CACHE_VERSION: "2025-08-21"
+ - name: Setup Python
+ uses: actions/setup-python@a26af69be951a213d495a4c3e4e4022e16d87065 # v5.6.0
Review Comment:
```suggestion
uses: actions/setup-python@e797f83bcb11b83ae66e0230d6156d7c80228e7c # v6.0.0
```
##########
cpp/src/arrow/flight/meson.build:
##########
@@ -44,7 +44,20 @@ install_headers(
subdir: 'arrow/flight',
)
-grpc_dep = dependency('grpc++')
+grpc_opts = []
+if cpp_compiler.get_id() == 'msvc'
+ grpc_opts += [
+ 'openssl:asm=disabled', # asm cannot compile on Windows
+ 're2:default_library=static', # re2 does not export symbols
+ ]
+endif
+
+grpc_cpp_dep = dependency(
+ 'gRPC++',
Review Comment:
pkg-config uses `grpc++.pc`.
```suggestion
'grpc++',
'gRPC++',
```
##########
.github/workflows/cpp_extra.yml:
##########
@@ -278,3 +279,70 @@ jobs:
cmake --build cpp/examples/minimal_build.build
cd cpp/examples/minimal_build
../minimal_build.build/arrow-example
+
+ meson-windows:
+ needs: check-labels
+ name: ${{ matrix.title }}
+ runs-on: ${{ matrix.runs-on }}
+ if: needs.check-labels.outputs.ci-extra == 'true'
+ timeout-minutes: 75
+ strategy:
+ fail-fast: false
+ matrix:
+ include:
+ - runs-on: windows-2022
+ title: AMD64 Windows Meson
Review Comment:
There is only one pattern. Can we simplify this? (Can we remove `strategy`?)
##########
cpp/src/arrow/meson.build:
##########
@@ -420,19 +481,38 @@ foreach key, val : arrow_components
arrow_deps += val.get('dependencies', [])
endforeach
+arrow_lib_cpp_shared_args = ['-DARROW_EXPORTING', '-DURI_LIBRARY_BUILD']
+if needs_compute
+ arrow_lib_cpp_shared_args += ['-DARROW_COMPUTE_EXPORTING']
+endif
+
arrow_lib = library(
'arrow',
sources: arrow_srcs,
include_directories: arrow_includes,
dependencies: arrow_deps,
install: true,
gnu_symbol_visibility: 'inlineshidden',
- cpp_shared_args: ['-DARROW_EXPORTING'],
+ gnu_symbol_visibility: 'hidden',
Review Comment:
Is this intentional change?
##########
ci/scripts/cpp_test.sh:
##########
@@ -95,6 +95,8 @@ if [ "${ARROW_USE_MESON:-OFF}" = "ON" ]; then
--no-rebuild \
--print-errorlogs \
--suite arrow \
+ --max-lines=0 \
+ --timeout-multiplier=10 \
Review Comment:
Could you sort them?
##########
cpp/src/arrow/meson.build:
##########
@@ -233,7 +287,14 @@ endif
if needs_snappy
arrow_util_srcs += ['util/compression_snappy.cc']
- arrow_util_deps += dependency('snappy')
+
Review Comment:
```suggestion
```
##########
.github/workflows/cpp_extra.yml:
##########
@@ -278,3 +279,70 @@ jobs:
cmake --build cpp/examples/minimal_build.build
cd cpp/examples/minimal_build
../minimal_build.build/arrow-example
+
+ meson-windows:
+ needs: check-labels
+ name: ${{ matrix.title }}
+ runs-on: ${{ matrix.runs-on }}
+ if: needs.check-labels.outputs.ci-extra == 'true'
+ timeout-minutes: 75
Review Comment:
It seems that `60` is enough because https://github.com/apache/arrow/actions/runs/17567392124/job/49896883950?pr=47282 completed in 43m 44s.
##########
.github/workflows/cpp_extra.yml:
##########
@@ -278,3 +279,70 @@ jobs:
cmake --build cpp/examples/minimal_build.build
cd cpp/examples/minimal_build
../minimal_build.build/arrow-example
+
+ meson-windows:
+ needs: check-labels
+ name: ${{ matrix.title }}
+ runs-on: ${{ matrix.runs-on }}
+ if: needs.check-labels.outputs.ci-extra == 'true'
+ timeout-minutes: 75
+ strategy:
+ fail-fast: false
+ matrix:
+ include:
+ - runs-on: windows-2022
+ title: AMD64 Windows Meson
+ steps:
+ - name: Checkout Arrow
+ uses: actions/checkout@v4
Review Comment:
```suggestion
uses: actions/checkout@08c6903cd8c0fde910a37f88322edcfb5dd907a8 # v5.0.0
```
##########
cpp/src/arrow/testing/process.h:
##########
@@ -39,6 +39,14 @@ class ARROW_TESTING_EXPORT Process {
bool IsRunning();
uint64_t pid();
+ static constexpr bool IsAvailable() {
+#ifdef BOOST_PROCESS_AVAILABLE
+ return true;
+#else
+ return false;
+#endif
Review Comment:
Could you define this in `process.cc` because `BOOST_PROCESS_AVAILABLE` is defined in `process.cc`?
##########
cpp/src/arrow/meson.build:
##########
@@ -420,19 +481,38 @@ foreach key, val : arrow_components
arrow_deps += val.get('dependencies', [])
endforeach
+arrow_lib_cpp_shared_args = ['-DARROW_EXPORTING', '-DURI_LIBRARY_BUILD']
+if needs_compute
+ arrow_lib_cpp_shared_args += ['-DARROW_COMPUTE_EXPORTING']
+endif
+
arrow_lib = library(
'arrow',
sources: arrow_srcs,
include_directories: arrow_includes,
dependencies: arrow_deps,
install: true,
gnu_symbol_visibility: 'inlineshidden',
- cpp_shared_args: ['-DARROW_EXPORTING'],
+ gnu_symbol_visibility: 'hidden',
+ c_shared_args: [
+ '-DARROW_EXPORTING',
+ '-DURI_LIBRARY_BUILD',
+ '-DURI_VISIBILITY',
Review Comment:
Do we need this for C++ too?
--
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: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] GH-46797: [C++] Support Meson on Windows [arrow]
Posted by "WillAyd (via GitHub)" <gi...@apache.org>.
WillAyd commented on PR #47282:
URL: https://github.com/apache/arrow/pull/47282#issuecomment-3268092123
OK merged in main so this is a lot cleaner now. @kou what do you think?
--
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: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] GH-46797: [C++] Support Meson on Windows [arrow]
Posted by "WillAyd (via GitHub)" <gi...@apache.org>.
WillAyd commented on code in PR #47282:
URL: https://github.com/apache/arrow/pull/47282#discussion_r2333872204
##########
cpp/src/arrow/testing/process.h:
##########
@@ -39,6 +39,14 @@ class ARROW_TESTING_EXPORT Process {
bool IsRunning();
uint64_t pid();
+ static constexpr bool IsAvailable() {
+#ifdef BOOST_PROCESS_AVAILABLE
+ return true;
+#else
+ return false;
+#endif
Review Comment:
Done!
--
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: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Meson win support [arrow]
Posted by "kou (via GitHub)" <gi...@apache.org>.
kou commented on code in PR #47282:
URL: https://github.com/apache/arrow/pull/47282#discussion_r2261978722
##########
.github/workflows/cpp_extra.yml:
##########
@@ -166,3 +166,58 @@ jobs:
ARCHERY_DOCKER_PASSWORD: ${{ secrets.DOCKERHUB_TOKEN }}
continue-on-error: true
run: archery docker push ${{ matrix.image }}
+
+ meson-windows:
+ needs: check-labels
+ name: ${{ matrix.title }}
+ runs-on: ${{ matrix.runs-on }}
+ if: needs.check-labels.outputs.ci-extra == 'true'
+ timeout-minutes: 75
+ strategy:
+ fail-fast: false
+ matrix:
+ include:
+ - runs-on: windows-2022
+ title: AMD64 Windows Meson
+ steps:
+ - name: Checkout Arrow
+ uses: actions/checkout@v4
+ with:
+ fetch-depth: 0
+ submodules: recursive
+ - name: Download Timezone Database
+ shell: bash
+ run: ci/scripts/download_tz_database.sh
+ - 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: cpp-ccache-windows-${{ env.CACHE_VERSION }}-${{ hashFiles('cpp/**') }}
+ restore-keys: cpp-ccache-windows-${{ env.CACHE_VERSION }}-
+ env:
+ # We can invalidate the current cache by updating this.
+ CACHE_VERSION: "2022-09-13"
+ - name: Build
+ shell: cmd
Review Comment:
```suggestion
shell: cmd
env:
ARROW_USE_MESON: ON
```
##########
cpp/src/arrow/meson.build:
##########
@@ -15,9 +15,20 @@
# specific language governing permissions and limitations
# under the License.
-dl_dep = dependency('dl')
+if host_machine.system() != 'windows'
+ dl_dep = dependency('dl')
+else
+ dl_dep = declare_dependency()
+endif
Review Comment:
Could you swap these branches for easy to read? (In general, no "not" is easier to read.)
```suggestion
if host_machine.system() == 'windows'
dl_dep = declare_dependency()
else
dl_dep = dependency('dl')
endif
```
##########
.github/workflows/cpp_extra.yml:
##########
@@ -166,3 +166,58 @@ jobs:
ARCHERY_DOCKER_PASSWORD: ${{ secrets.DOCKERHUB_TOKEN }}
continue-on-error: true
run: archery docker push ${{ matrix.image }}
+
+ meson-windows:
+ needs: check-labels
+ name: ${{ matrix.title }}
+ runs-on: ${{ matrix.runs-on }}
+ if: needs.check-labels.outputs.ci-extra == 'true'
+ timeout-minutes: 75
+ strategy:
+ fail-fast: false
+ matrix:
+ include:
+ - runs-on: windows-2022
+ title: AMD64 Windows Meson
+ steps:
+ - name: Checkout Arrow
+ uses: actions/checkout@v4
+ with:
+ fetch-depth: 0
+ submodules: recursive
+ - name: Download Timezone Database
+ shell: bash
+ run: ci/scripts/download_tz_database.sh
+ - 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: cpp-ccache-windows-${{ env.CACHE_VERSION }}-${{ hashFiles('cpp/**') }}
+ restore-keys: cpp-ccache-windows-${{ env.CACHE_VERSION }}-
+ env:
+ # We can invalidate the current cache by updating this.
+ CACHE_VERSION: "2022-09-13"
+ - name: Build
+ shell: cmd
+ run: |
+ call "C:\Program Files\Microsoft Visual Studio\2022\Enterprise\VC\Auxiliary\Build\vcvarsall.bat" x64
+ bash -c "ARROW_USE_MESON=ON; ci/scripts/cpp_build.sh $(pwd) $(pwd)/build ${{ matrix.run-options || '' }}"
Review Comment:
```suggestion
bash -c "ci/scripts/cpp_build.sh $(pwd) $(pwd)/build"
```
##########
cpp/src/arrow/util/meson.build:
##########
@@ -224,18 +222,25 @@ utility_test_srcs = [
]
if host_machine.system() == 'windows'
+ # meson does not natively support manifest files like CMake does
Review Comment:
https://github.com/mesonbuild/meson/issues/13041 may be related.
`/MANIFEST:EMBED /MANIFESTINPUT:io_util_test.manifest` linker options may work.
See also: https://learn.microsoft.com/en-us/cpp/build/reference/manifestinput-specify-manifest-input?view=msvc-170
##########
cpp/src/arrow/meson.build:
##########
@@ -484,14 +505,22 @@ if needs_compute
arrow_compute_lib = library(
'arrow-compute',
sources: arrow_compute_lib_sources,
- dependencies: arrow_dep,
+ dependencies: [arrow_dep] + int128_deps,
Review Comment:
Hmm. Why do we need this?
If `libarrow` is a static library, it must have `int128_deps` dependency. `libarrow` consumers should not care about `int128_deps`.
##########
cpp/src/arrow/compute/kernels/ree_util_internal.h:
##########
@@ -347,7 +347,7 @@ Result<std::shared_ptr<ArrayData>> PreallocateRunEndsArray(
/// \param has_validity_buffer a validity buffer must be allocated
/// \param length the length of the values array
/// \param data_buffer_size the size of the data buffer for string and binary types
-ARROW_COMPUTE_EXPORT Result<std::shared_ptr<ArrayData>> PreallocateValuesArray(
+ARROW_EXPORT Result<std::shared_ptr<ArrayData>> PreallocateValuesArray(
Review Comment:
Really?
It seems that it's used by `libarrow_compute` not `libarrow`: https://github.com/apache/arrow/blob/97d7cdb9daef4f0e2bd050b2c032413d8697ae02/cpp/src/arrow/CMakeLists.txt#L767
##########
cpp/src/arrow/meson.build:
##########
@@ -423,12 +435,21 @@ arrow_lib = library(
dependencies: arrow_deps,
install: true,
gnu_symbol_visibility: 'hidden',
- cpp_shared_args: ['-DARROW_EXPORTING'],
+ c_shared_args: ['-DARROW_EXPORTING', '-DURI_LIBRARY_BUILD', '-DURI_VISIBILITY'],
+ c_static_args: ['-DURI_STATIC_BUILD'],
+ cpp_shared_args: ['-DARROW_EXPORTING', '-DURI_LIBRARY_BUILD'],
+ cpp_static_args: ['-DARROW_STATIC'],
)
+arrow_compile_args = []
+if host_machine.system() == 'windows' and get_option('default_library') != 'shared'
Review Comment:
We can always define `ARROW_STATIC`:
```suggestion
if get_option('default_library') != 'shared'
```
BTW, does this work with `default_library=both`?
##########
.github/workflows/cpp_extra.yml:
##########
@@ -166,3 +166,58 @@ jobs:
ARCHERY_DOCKER_PASSWORD: ${{ secrets.DOCKERHUB_TOKEN }}
continue-on-error: true
run: archery docker push ${{ matrix.image }}
+
+ meson-windows:
+ needs: check-labels
+ name: ${{ matrix.title }}
+ runs-on: ${{ matrix.runs-on }}
+ if: needs.check-labels.outputs.ci-extra == 'true'
+ timeout-minutes: 75
+ strategy:
+ fail-fast: false
+ matrix:
+ include:
+ - runs-on: windows-2022
+ title: AMD64 Windows Meson
+ steps:
+ - name: Checkout Arrow
+ uses: actions/checkout@v4
+ with:
+ fetch-depth: 0
+ submodules: recursive
+ - name: Download Timezone Database
+ shell: bash
+ run: ci/scripts/download_tz_database.sh
+ - 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: cpp-ccache-windows-${{ env.CACHE_VERSION }}-${{ hashFiles('cpp/**') }}
+ restore-keys: cpp-ccache-windows-${{ env.CACHE_VERSION }}-
Review Comment:
```suggestion
key: cpp-ccache-windows-meson-${{ env.CACHE_VERSION }}-${{ hashFiles('cpp/**') }}
restore-keys: cpp-ccache-windows-meson-${{ env.CACHE_VERSION }}-
```
##########
.github/workflows/cpp_extra.yml:
##########
@@ -166,3 +166,58 @@ jobs:
ARCHERY_DOCKER_PASSWORD: ${{ secrets.DOCKERHUB_TOKEN }}
continue-on-error: true
run: archery docker push ${{ matrix.image }}
+
+ meson-windows:
+ needs: check-labels
+ name: ${{ matrix.title }}
+ runs-on: ${{ matrix.runs-on }}
+ if: needs.check-labels.outputs.ci-extra == 'true'
+ timeout-minutes: 75
+ strategy:
+ fail-fast: false
+ matrix:
+ include:
+ - runs-on: windows-2022
+ title: AMD64 Windows Meson
+ steps:
+ - name: Checkout Arrow
+ uses: actions/checkout@v4
+ with:
+ fetch-depth: 0
+ submodules: recursive
+ - name: Download Timezone Database
+ shell: bash
+ run: ci/scripts/download_tz_database.sh
+ - 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: cpp-ccache-windows-${{ env.CACHE_VERSION }}-${{ hashFiles('cpp/**') }}
+ restore-keys: cpp-ccache-windows-${{ env.CACHE_VERSION }}-
+ env:
+ # We can invalidate the current cache by updating this.
+ CACHE_VERSION: "2022-09-13"
+ - name: Build
+ shell: cmd
+ run: |
+ call "C:\Program Files\Microsoft Visual Studio\2022\Enterprise\VC\Auxiliary\Build\vcvarsall.bat" x64
+ bash -c "ARROW_USE_MESON=ON; ci/scripts/cpp_build.sh $(pwd) $(pwd)/build ${{ matrix.run-options || '' }}"
+ - name: Test
+ shell: bash
Review Comment:
```suggestion
shell: bash
env:
ARROW_USE_MESON: ON
```
##########
.github/workflows/cpp_extra.yml:
##########
@@ -166,3 +166,58 @@ jobs:
ARCHERY_DOCKER_PASSWORD: ${{ secrets.DOCKERHUB_TOKEN }}
continue-on-error: true
run: archery docker push ${{ matrix.image }}
+
+ meson-windows:
+ needs: check-labels
+ name: ${{ matrix.title }}
+ runs-on: ${{ matrix.runs-on }}
+ if: needs.check-labels.outputs.ci-extra == 'true'
+ timeout-minutes: 75
+ strategy:
+ fail-fast: false
+ matrix:
+ include:
+ - runs-on: windows-2022
+ title: AMD64 Windows Meson
+ steps:
+ - name: Checkout Arrow
+ uses: actions/checkout@v4
+ with:
+ fetch-depth: 0
+ submodules: recursive
+ - name: Download Timezone Database
+ shell: bash
+ run: ci/scripts/download_tz_database.sh
+ - 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: cpp-ccache-windows-${{ env.CACHE_VERSION }}-${{ hashFiles('cpp/**') }}
+ restore-keys: cpp-ccache-windows-${{ env.CACHE_VERSION }}-
+ env:
+ # We can invalidate the current cache by updating this.
+ CACHE_VERSION: "2022-09-13"
+ - name: Build
+ shell: cmd
+ run: |
+ call "C:\Program Files\Microsoft Visual Studio\2022\Enterprise\VC\Auxiliary\Build\vcvarsall.bat" x64
+ bash -c "ARROW_USE_MESON=ON; ci/scripts/cpp_build.sh $(pwd) $(pwd)/build ${{ matrix.run-options || '' }}"
+ - name: Test
+ shell: bash
+ run: |
+ # For ORC
+ export TZDIR=/c/msys64/usr/share/zoneinfo
+ ARROW_USE_MESON=ON ci/scripts/cpp_test.sh $(pwd) $(pwd)/build ${{ matrix.run-options || '' }}
Review Comment:
```suggestion
ci/scripts/cpp_test.sh $(pwd) $(pwd)/build
```
--
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: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] GH-46797: [C++] Support Meson on Windows [arrow]
Posted by "WillAyd (via GitHub)" <gi...@apache.org>.
WillAyd commented on PR #47282:
URL: https://github.com/apache/arrow/pull/47282#issuecomment-3167925890
> Could you open an issue for this?
Sorry about that - there is an open issue I just messed up the commit name. I've updated the title now and can squash on next push
--
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: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Meson win support [arrow]
Posted by "github-actions[bot] (via GitHub)" <gi...@apache.org>.
github-actions[bot] commented on PR #47282:
URL: https://github.com/apache/arrow/pull/47282#issuecomment-3165589915
<!--
Licensed to the Apache Software Foundation (ASF) under one
or more contributor license agreements. See the NOTICE file
distributed with this work for additional information
regarding copyright ownership. The ASF licenses this file
to you under the Apache License, Version 2.0 (the
"License"); you may not use this file except in compliance
with the License. You may obtain a copy of the License at
http://www.apache.org/licenses/LICENSE-2.0
Unless required by applicable law or agreed to in writing,
software distributed under the License is distributed on an
"AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
KIND, either express or implied. See the License for the
specific language governing permissions and limitations
under the License.
-->
Thanks for opening a pull request!
If this is not a [minor PR](https://github.com/apache/arrow/blob/main/CONTRIBUTING.md#Minor-Fixes). Could you open an issue for this pull request on GitHub? https://github.com/apache/arrow/issues/new/choose
Opening GitHub issues ahead of time contributes to the [Openness](http://theapacheway.com/open/#:~:text=Openness%20allows%20new%20users%20the,must%20happen%20in%20the%20open.) of the Apache Arrow project.
Then could you also rename the pull request title in the following format?
GH-${GITHUB_ISSUE_ID}: [${COMPONENT}] ${SUMMARY}
or
MINOR: [${COMPONENT}] ${SUMMARY}
See also:
* [Other pull requests](https://github.com/apache/arrow/pulls/)
* [Contribution Guidelines - Contributing Overview](https://arrow.apache.org/docs/developers/overview.html)
--
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: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] GH-46797: [C++] Support Meson on Windows [arrow]
Posted by "WillAyd (via GitHub)" <gi...@apache.org>.
WillAyd commented on code in PR #47282:
URL: https://github.com/apache/arrow/pull/47282#discussion_r2263252445
##########
.github/workflows/cpp_extra.yml:
##########
@@ -166,3 +166,73 @@ jobs:
ARCHERY_DOCKER_PASSWORD: ${{ secrets.DOCKERHUB_TOKEN }}
continue-on-error: true
run: archery docker push ${{ matrix.image }}
+
+ meson-windows:
+ needs: check-labels
+ name: ${{ matrix.title }}
+ runs-on: ${{ matrix.runs-on }}
+ if: needs.check-labels.outputs.ci-extra == 'true'
+ timeout-minutes: 75
+ strategy:
+ fail-fast: false
+ matrix:
+ include:
+ - runs-on: windows-2022
+ title: AMD64 Windows Meson
+ steps:
+ - name: Checkout Arrow
+ uses: actions/checkout@v4
+ with:
+ fetch-depth: 0
+ submodules: recursive
+ - name: Download Timezone Database
+ shell: bash
+ run: ci/scripts/download_tz_database.sh
+ - 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: cpp-ccache-windows-meson-${{ env.CACHE_VERSION }}-${{ hashFiles('cpp/**') }}
+ restore-keys: cpp-ccache-windows-meson-${{ env.CACHE_VERSION }}-
+ env:
+ # We can invalidate the current cache by updating this.
+ CACHE_VERSION: "2022-09-13"
+ - name: Setup Python
+ uses: actions/setup-python@a26af69be951a213d495a4c3e4e4022e16d87065 # v5.6.0
+ with:
+ python-version: 3
+ - name: Install Meson
+ run: |
+ python3 -m pip install meson
+ - name: Build
+ shell: cmd
+ env:
+ ARROW_USE_MESON: ON
+ ARROW_HOME: C:\arrow-dist
+ run: |
+ echo %PATH%
+ call "C:\Program Files\Microsoft Visual Studio\2022\Enterprise\VC\Auxiliary\Build\vcvarsall.bat" x64
+ echo %PATH%
+ dir "C:\Program Files\Microsoft Visual Studio\2022\Enterprise\VC\Tools\MSVC\14.44.35207\bin\HostX64\x64"
+ bash -c "ci/scripts/cpp_build.sh $(pwd) $(pwd)/build ${{ matrix.run-options || '' }}"
Review Comment:
I believe the current error has to do with the fact that calling `bash -c link ..." invokes the linker that comes bundled with bash, and not the linker that the vcvarsall.bat script is supposed to activate. You can reproduce this locally:
```cmd
> link /help
Microsoft (R) Incremental Linker Version 14.44.35214.0
Copyright (C) Microsoft Corporation. All rights reserved.
For help on Linker, type `link /link' or `link'
For help on Library Manager, type `link /lib' or `lib'
For help on Dumper, type `link /dump' or `dumpbin'
For help on Editor, type `link /edit' or `editbin'
For help on CvtCIL, type `link /cvtcil'
For help on PushThunkObj Generator, type `link /pushthunkobj'
> bash -c "link /help"
link: missing operand after ‘/help’
Try 'link --help' for more information.
> bash -c "link --help"
Usage: link FILE1 FILE2
or: link OPTION
Call the link function to create a link named FILE2 to an existing FILE1.
--help display this help and exit
--version output version information and exit
GNU coreutils online help: <https://www.gnu.org/software/coreutils/>
Report any translation bugs to <https://translationproject.org/team/>
Full documentation <https://www.gnu.org/software/coreutils/link>
or available locally via: info '(coreutils) link invocation'
```
I guess the current CMake configuration doesn't care about this, although it makes sense for Meson to be suspect of this environment.
Maybe its worth creating a separate `cpp_build.bat` script for Windows platforms to use? What do you think @kou ?
--
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: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] GH-46797: [C++] Support Meson on Windows [arrow]
Posted by "github-actions[bot] (via GitHub)" <gi...@apache.org>.
github-actions[bot] commented on PR #47282:
URL: https://github.com/apache/arrow/pull/47282#issuecomment-6383569291
Thank you for your contribution. Unfortunately, this pull request has been marked as stale because it has had no activity in the past 365 days. Please remove the stale label or comment below, or this PR will be closed in 14 days. Feel free to re-open this if it has been closed in error. If you do not have repository permissions to reopen the PR, please tag a maintainer.
--
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: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org