-
Notifications
You must be signed in to change notification settings - Fork 40
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
Add Meson build with Werror #448
Changes from 24 commits
f3a0ea5
8d82f7c
6c7da11
63c2231
80c5923
d204fb1
05089a9
6a29031
43248be
4562863
0d91852
620d6a6
bc6291e
9f52be1
b22e567
d0fab89
bf6b20f
f149708
fd5fb7d
85dbccf
55b98f1
61675a8
aa0c74b
2ea48b4
0dfa1e0
3472ae2
40b4cd5
23b3c9c
e1d3306
6ae24e9
dddd193
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
This file was deleted.
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -45,10 +45,10 @@ ARROW_CPP_SCRATCH_DIR="arrow-cpp-build-${ARROW_CPP_VERSION}" | |
mkdir "${ARROW_CPP_SCRATCH_DIR}" | ||
pushd "${ARROW_CPP_SCRATCH_DIR}" | ||
|
||
curl -L "https://www.apache.org/dyn/closer.lua?action=download&filename=arrow/arrow-${ARROW_CPP_VERSION}/apache-arrow-${ARROW_CPP_VERSION}.tar.gz" | \ | ||
curl -L "https://github.com/apache/arrow/archive/refs/heads/main.tar.gz" | \ | ||
tar -zxf - | ||
mkdir build && cd build | ||
cmake ../apache-arrow-${ARROW_CPP_VERSION}/cpp \ | ||
cmake ../arrow-main/cpp \ | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Will the Arrow 17.0.0 release include the upstream changes required to make this work? We can't merge this particular change but we can update the version we use in CI once released. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Don't think so. I fixed a few but they keep popping up, especially after having added metal / ipc support. Can keep trying but I think unless Arrow comprehensively wanted to fix these warnings its going to be hard downstream to keep this clean An alternate solution may be to treat Arrow as a system include in the build systems so that it doesn't show any warnings from that library. That might allow us to at least keep this clean within nanoarrow There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I'll pipe the Arrow C++ removal through in the next few weeks. We've let that dependency complicate quite a lot of things and I've mentioned its hypothetical removal quite a lot of times. |
||
-DCMAKE_BUILD_TYPE=Debug \ | ||
-DARROW_JEMALLOC=OFF \ | ||
-DARROW_SIMD_LEVEL=NONE \ | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -30,7 +30,7 @@ project( | |
) | ||
|
||
if meson.get_compiler('cpp').get_id() == 'gcc' or meson.get_compiler('cpp').get_id() == 'clang' | ||
add_project_arguments(['-fvisibility=hidden'], language: 'cpp') | ||
add_project_arguments(['-fvisibility=hidden', '-Wno-unused-parameter'], language: 'cpp') | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Since these aren't done upstream in Arrow kind of hard to keep things synced; probably just easiest to ignore until we potentially drop the Arrow testing dependency |
||
endif | ||
|
||
nanoarrow_dep_args = [] | ||
|
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.
These changes are not permanent - just setting them up to see what we need for a clean build of Arrow.