-
Notifications
You must be signed in to change notification settings - Fork 3.6k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
GH-45589: [C++] Enable singular test in Meson configuration #45596
base: main
Are you sure you want to change the base?
Conversation
|
gmock_dep = disabler() | ||
endif | ||
|
||
arrow_test_lib = static_library( |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The static_library is required here because the Meson wrap dependency for gtest only provides consumers with the source files, and when using that in two different shared libraries then linking together you get an ODR violation.
See also mesonbuild/wrapdb#1937
@@ -56,4 +56,10 @@ if git_description == '' | |||
git_description = run_command('git', 'describe', '--tags', check: false).stdout().strip() | |||
endif | |||
|
|||
needs_integration = false |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is to follow along with the CMake configuration. The option doesn't yet exist in Meson so I've hard-coded for now
meson setup \ | ||
--prefix=${MESON_PREFIX:-${ARROW_HOME}} \ | ||
--buildtype=${ARROW_BUILD_TYPE:-debug} \ | ||
-Dtests=true \ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
meson setup \ | |
--prefix=${MESON_PREFIX:-${ARROW_HOME}} \ | |
--buildtype=${ARROW_BUILD_TYPE:-debug} \ | |
-Dtests=true \ | |
function meson_boolean() { | |
if [ "${1}" = "ON" ]; then | |
echo "true" | |
else | |
echo "false" | |
fi | |
} | |
meson setup \ | |
--prefix=${MESON_PREFIX:-${ARROW_HOME}} \ | |
--buildtype=${ARROW_BUILD_TYPE:-debug} \ | |
-Dtests=$(meson_boolean ${ARROW_BUILD_TESTS:-OFF}) \ |
subprojects/* | ||
!subprojects/packagefiles | ||
!subprojects/*.wrap |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
subprojects/* | |
!subprojects/packagefiles | |
!subprojects/*.wrap | |
/subprojects/* | |
!/subprojects/packagefiles | |
!/subprojects/*.wrap |
@github-actions crossbow submit test-conda-cpp-meson |
Revision: 65779fa Submitted crossbow builds: ursacomputing/crossbow @ actions-b0685743f1
|
https://github.com/ursacomputing/crossbow/actions/runs/13468923214/job/37639840316#step:6:448
Hmm. It's strange that RapidJSON wasn't found. RapidJSON exists in the environment: https://github.com/ursacomputing/crossbow/actions/runs/13468923214/job/37639840316#step:6:351
|
Sorry I've been away for a few days.
In spite of Meson not finding the system RapidJSON, it does still fall back to using the wrap file to download an appropriate copy. Seems like the error message from that run has to do with resolving the filepath to the bundled flatbuffers. Will investigate further |
FWIW it might even be worthwhile to use Meson's wrap and subproject systems for flatbuffers instead of using a vendored copy. The limiting factor was that Arrow seems to statically assert the required major/minor version of flatbuffers, and the minor version requested does not exist in the wrapdb. If there is some flexibility on that version check we could go down that path |
We should add support for system Flatbuffers, right? (We can fallback to the vendored Flatbuffers and generated files when system Flatbuffers don't exist.) Could you open an issue for this? Let's discuss this in the new issue. |
Rationale for this change
Adding this singular test helps us make incremental progress on Meson as a build system generator
What changes are included in this PR?
A single array-test for libarrow is being added
Are these changes tested?
Yes - see CI
Are there any user-facing changes?
No - this is only for developers