paleolimbot commented on code in PR #413: URL: https://github.com/apache/arrow-nanoarrow/pull/413#discussion_r1548107091
########## meson.build: ########## @@ -0,0 +1,50 @@ +# 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. + +project( + 'nanoarrow', + 'c', 'cpp', + version: '0.5.0-SNAPSHOT', Review Comment: Since this hard-codes the version, it should probably be added to the version bumper: https://github.com/apache/arrow-nanoarrow/blob/a9ef7459dec2da8a28b4e00fd9cec5b825905432/dev/release/utils-prepare.sh#L46 ########## examples/meson-minimal/src/app.c: ########## Review Comment: There is a more minimal example in the cmake build tests folder in `examples/` that might be a better fit here (and also might be a better fit in the minimal cmake example, too, in some future PR making all our examples better). ########## examples/meson-minimal/subprojects/nanoarrow.wrap: ########## @@ -0,0 +1,23 @@ +# 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. + Review Comment: Does this need a comment explaining what the difference would be if this weren't in the source tree? (At least until nanoarrow is in the wrap database?) ########## ci/scripts/build-with-meson.sh: ########## @@ -0,0 +1,103 @@ +#!/usr/bin/env bash +# +# 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. + +set -e +set -o pipefail + +if [ ${VERBOSE:-0} -gt 0 ]; then + set -x +fi + +SOURCE_DIR="$(cd "$(dirname "${BASH_SOURCE[0]:-$0}")" && pwd)" +NANOARROW_DIR="$(cd "${SOURCE_DIR}/../.." && pwd)" + +show_header() { + if [ -z "$GITHUB_ACTIONS" ]; then + echo "" + printf '=%.0s' $(seq ${#1}); printf '\n' + echo "${1}" + printf '=%.0s' $(seq ${#1}); printf '\n' + else + echo "::group::${1}"; printf '\n' + fi +} + + +case $# in + 0) TARGET_NANOARROW_DIR="${NANOARROW_DIR}" + ;; + 1) TARGET_NANOARROW_DIR="$1" + ;; + *) echo "Usage:" + echo " Build nanoarrow based on a source checkout elsewhere:" + echo " $0 path/to/arrow-nanoarrow" + exit 1 + ;; +esac + +function main() { + pushd ${TARGET_NANOARROW_DIR} + + SANDBOX_DIR="_meson_builddir" + if [ -d "${SANDBOX_DIR}" ]; then + rm -rf "${SANDBOX_DIR}" + fi + mkdir "${SANDBOX_DIR}" + + SUBPROJ_DIR="subprojects" + if [ -d "${SUBPROJ_DIR}" ]; then + rm -rf "${SUBPROJ_DIR}" + fi + mkdir "${SUBPROJ_DIR}" + + show_header "Install subprojects" + meson wrap install gtest + meson wrap install google-benchmark + meson wrap install nlohmann_json + + show_header "Compile project with meson" + meson setup "${SANDBOX_DIR}" --pkg-config-path $PKG_CONFIG_PATH Review Comment: It seems like this is for Arrow C++, and that's a test dependency I'd like to remove in the near future anyway (we *almost* have the integration tests running which are better suited to testing that kind of thing). ########## .github/workflows/meson-build.yaml: ########## @@ -0,0 +1,56 @@ +# 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. + +name: Meson Build Testing + +on: + schedule: + - cron: '5 0 * * 0' + pull_request: + branches: + - main + paths: + - '.github/workflows/meson-build.yaml' + - 'ci/scripts/build-with-meson.sh' Review Comment: Probably this should also run when any of the config or options or build files are modified? ########## src/nanoarrow/meson.build: ########## @@ -0,0 +1,128 @@ +# 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. + +conf_data = configuration_data() + +ns = get_option('NANOARROW_NAMESPACE') +conf_data.set('NANOARROW_NAMESPACE_DEFINE', '#define NANOARROW_NAMESPACE ' + ns) + +# TODO: CMake uses a regex to extract major/minor/patch from this string +version = '0.5.0-SNAPSHOT' +ver_major = 0 +ver_minor = 5 +ver_patch = 0 +conf_data.set('NANOARROW_VERSION', version) +conf_data.set('NANOARROW_VERSION_MAJOR', ver_major) +conf_data.set('NANOARROW_VERSION_MINOR', ver_minor) +conf_data.set('NANOARROW_VERSION_PATCH', ver_patch) + +configure_file(input: 'nanoarrow_config.h.in', + output: 'nanoarrow_config.h', + configuration: conf_data) + +nanoarrow_lib = library( + 'nanoarrow', + 'array.c', + 'schema.c', + 'array_stream.c', + 'utils.c', + install: true) + +curdir = include_directories('.') # only needed when used as subproject? +incdir = include_directories('..') + +nanoarrow_dep = declare_dependency(include_directories: [curdir, incdir], + link_with: nanoarrow_lib) + +if get_option('NANOARROW_BUILD_TESTS') or get_option('NANOARROW_BUILD_INTEGRATION_TESTS') + nlohmann_json_dep = dependency('nlohmann_json') + + c_data_integration_lib = library('nanoarrow_c_data_integration', + 'integration/c_data_integration.cc', + link_with: nanoarrow_lib, + dependencies: [nlohmann_json_dep], + include_directories: incdir) + +endif + +if get_option('NANOARROW_BUILD_TESTS') + # CMake configuration sets MEMORYCHECK_COMMAND_OPTIONS but with meson you instead + # wrap the tests with valgrind via `meson test --wrap=valgrind`. See + # https://mesonbuild.com/Unit-tests.html + + # Similarly code coverage has a built in option users should use instead + # https://mesonbuild.com/Unit-tests.html#coverage + + arrow_dep = dependency('arrow') + gtest_dep = dependency('gtest', fallback: ['gtest', 'gtest_main_dep']) + + # TODO: the CMake configuration sets a different C++ version depending on + # the version of Arrow used. Meson allows this standard to be set by subproject + # do we need to do anything here? + # https://mesonbuild.com/Builtin-options.html#specifying-options-per-subproject + + utils_test = executable('utils_test', 'utils_test.cc', + link_with: nanoarrow_lib, + dependencies: [arrow_dep, gtest_dep], + include_directories: incdir) + test('utils test', utils_test) + + buffer_test = executable('buffer_test', 'buffer_test.cc', + dependencies: [arrow_dep, gtest_dep], + link_with: nanoarrow_lib, + include_directories: incdir) + test('buffer test', buffer_test) + + array_test = executable('array_test', 'array_test.cc', + dependencies: [arrow_dep, gtest_dep], + link_with: nanoarrow_lib, + include_directories: incdir) + test('array test', array_test) + + schema_test = executable('schema_test', 'schema_test.cc', + dependencies: [arrow_dep, gtest_dep], + link_with: nanoarrow_lib, + include_directories: incdir) + test('schema test', schema_test) + + array_stream_test = executable('array_stream_test', 'array_stream_test.cc', + dependencies: [arrow_dep, gtest_dep], + link_with: nanoarrow_lib, + include_directories: incdir) + test('array_stream test', array_stream_test) + + nanoarrow_hpp_test = executable('nanoarrow_hpp_test', 'nanoarrow_hpp_test.cc', + dependencies: [arrow_dep, gtest_dep], + link_with: nanoarrow_lib, + include_directories: incdir) + test('nanoarrow_hpp test', nanoarrow_hpp_test) + + nlohmann_json_dep = dependency('nlohmann_json') + nanoarrow_testing_test = executable('nanoarrow_testing_test', 'nanoarrow_testing_test.cc', + dependencies: [arrow_dep, gtest_dep, nlohmann_json_dep], + link_with: nanoarrow_lib, + include_directories: incdir) + test('nanoarrow_testing test', nanoarrow_testing_test) + + + c_data_integration_test = executable('c_data_integration_test', 'integration/c_data_integration_test.cc', + link_with: c_data_integration_lib, + dependencies: [arrow_dep, gtest_dep], + include_directories: incdir) + test('c_data_integration test', c_data_integration_test) + Review Comment: Is it worth putting a comment in the `CMakeLists.txt` that the list of tests should be updated here, too? ########## src/nanoarrow/meson.build: ########## @@ -0,0 +1,128 @@ +# 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. + +conf_data = configuration_data() + +ns = get_option('NANOARROW_NAMESPACE') +conf_data.set('NANOARROW_NAMESPACE_DEFINE', '#define NANOARROW_NAMESPACE ' + ns) + +# TODO: CMake uses a regex to extract major/minor/patch from this string +version = '0.5.0-SNAPSHOT' Review Comment: This can be hard-coded as long as it's in the version-bumping script (linked above) ########## ci/scripts/build-with-meson.sh: ########## @@ -0,0 +1,103 @@ +#!/usr/bin/env bash +# +# 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. + +set -e +set -o pipefail + +if [ ${VERBOSE:-0} -gt 0 ]; then + set -x +fi + +SOURCE_DIR="$(cd "$(dirname "${BASH_SOURCE[0]:-$0}")" && pwd)" +NANOARROW_DIR="$(cd "${SOURCE_DIR}/../.." && pwd)" + +show_header() { + if [ -z "$GITHUB_ACTIONS" ]; then + echo "" + printf '=%.0s' $(seq ${#1}); printf '\n' + echo "${1}" + printf '=%.0s' $(seq ${#1}); printf '\n' + else + echo "::group::${1}"; printf '\n' + fi +} + + +case $# in + 0) TARGET_NANOARROW_DIR="${NANOARROW_DIR}" + ;; + 1) TARGET_NANOARROW_DIR="$1" + ;; + *) echo "Usage:" + echo " Build nanoarrow based on a source checkout elsewhere:" + echo " $0 path/to/arrow-nanoarrow" + exit 1 + ;; +esac + +function main() { + pushd ${TARGET_NANOARROW_DIR} + + SANDBOX_DIR="_meson_builddir" + if [ -d "${SANDBOX_DIR}" ]; then + rm -rf "${SANDBOX_DIR}" + fi + mkdir "${SANDBOX_DIR}" + + SUBPROJ_DIR="subprojects" + if [ -d "${SUBPROJ_DIR}" ]; then + rm -rf "${SUBPROJ_DIR}" + fi + mkdir "${SUBPROJ_DIR}" + + show_header "Install subprojects" + meson wrap install gtest + meson wrap install google-benchmark + meson wrap install nlohmann_json + + show_header "Compile project with meson" + meson setup "${SANDBOX_DIR}" --pkg-config-path $PKG_CONFIG_PATH + + pushd "${SANDBOX_DIR}" + + show_header "Run test suite" + meson configure -DNANOARROW_BUILD_TESTS=true \ + -Db_coverage=true + meson compile + meson test --wrap valgrind Review Comment: Google Benchmark defaults to running the tests in CMake, too (I remember having to explicitly disable). The benchmarking is definitely its own beast...the ability to build it from the top-level CMake just makes life easier when writing the benchmarks. ########## src/nanoarrow/meson.build: ########## @@ -0,0 +1,128 @@ +# 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. + +conf_data = configuration_data() + +ns = get_option('NANOARROW_NAMESPACE') +conf_data.set('NANOARROW_NAMESPACE_DEFINE', '#define NANOARROW_NAMESPACE ' + ns) + +# TODO: CMake uses a regex to extract major/minor/patch from this string +version = '0.5.0-SNAPSHOT' +ver_major = 0 +ver_minor = 5 +ver_patch = 0 +conf_data.set('NANOARROW_VERSION', version) +conf_data.set('NANOARROW_VERSION_MAJOR', ver_major) +conf_data.set('NANOARROW_VERSION_MINOR', ver_minor) +conf_data.set('NANOARROW_VERSION_PATCH', ver_patch) + +configure_file(input: 'nanoarrow_config.h.in', + output: 'nanoarrow_config.h', + configuration: conf_data) + +nanoarrow_lib = library( + 'nanoarrow', + 'array.c', + 'schema.c', + 'array_stream.c', + 'utils.c', + install: true) + +curdir = include_directories('.') # only needed when used as subproject? +incdir = include_directories('..') + +nanoarrow_dep = declare_dependency(include_directories: [curdir, incdir], + link_with: nanoarrow_lib) + +if get_option('NANOARROW_BUILD_TESTS') or get_option('NANOARROW_BUILD_INTEGRATION_TESTS') + nlohmann_json_dep = dependency('nlohmann_json') + + c_data_integration_lib = library('nanoarrow_c_data_integration', + 'integration/c_data_integration.cc', + link_with: nanoarrow_lib, + dependencies: [nlohmann_json_dep], + include_directories: incdir) + +endif + +if get_option('NANOARROW_BUILD_TESTS') + # CMake configuration sets MEMORYCHECK_COMMAND_OPTIONS but with meson you instead + # wrap the tests with valgrind via `meson test --wrap=valgrind`. See + # https://mesonbuild.com/Unit-tests.html + + # Similarly code coverage has a built in option users should use instead + # https://mesonbuild.com/Unit-tests.html#coverage + + arrow_dep = dependency('arrow') + gtest_dep = dependency('gtest', fallback: ['gtest', 'gtest_main_dep']) + + # TODO: the CMake configuration sets a different C++ version depending on + # the version of Arrow used. Meson allows this standard to be set by subproject + # do we need to do anything here? Review Comment: I don't think we need this for the Meson build...we need *some* way to make sure that this works on Centos7, but not for very long, and we can just use CMake to test that. -- 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: [email protected] For queries about this service, please contact Infrastructure at: [email protected]
