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 2024/06/03 21:41:16 UTC
[PR] feat: Meson Support for ADBC [arrow-adbc]
WillAyd opened a new pull request, #1904:
URL: https://github.com/apache/arrow-adbc/pull/1904
This worked pretty well in nanoarrow so figured worth trying in ADBC. In the long run arrow-adbc would make for a nice addition to the Meson WrapDB - it would be an easy way to install the source for a desired set of driver(s)
In the immediate term, Meson really helps simplify the current CMake hoops you have to jump through to wrangle dependencies, and I think is easier to use
--
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] feat: Meson Support for ADBC [arrow-adbc]
Posted by "kou (via GitHub)" <gi...@apache.org>.
kou commented on code in PR #1904:
URL: https://github.com/apache/arrow-adbc/pull/1904#discussion_r1626843592
##########
.gitignore:
##########
@@ -121,3 +121,7 @@ target/
/ci/linux-packages/yum/merged/
/ci/linux-packages/yum/repositories/
/ci/linux-packages/yum/tmp/
+
+# Meson subproject support
+c/subprojects/*
+!c/subprojects/*.wrap
Review Comment:
```suggestion
/c/subprojects/*
!/c/subprojects/*.wrap
```
##########
.github/workflows/native-unix.yml:
##########
@@ -206,6 +206,38 @@ jobs:
run: |
./ci/scripts/cpp_test.sh "$(pwd)/build"
+ drivers-test-meson:
+ name: "Meson - C/C++ (Conda/${{ matrix.os }})"
+ runs-on: ${{ matrix.os }}
+ strategy:
+ matrix:
+ os: ["ubuntu-latest"]
+ steps:
+ - uses: actions/checkout@v4
+ with:
+ fetch-depth: 0
+ persist-credentials: false
+ - name: Install Dependencies
+ run: |
+ sudo apt update
+ sudo apt install -y libpq-dev ninja-build
+ - name: Setup Python
+ uses: actions/setup-python@v5
+ with:
+ python-version: '3.x'
+ - name: Install Meson via Python
+ run: pip install meson
+ - name: Build
+ run: |
+ cd c
+ meson setup -Dpostgresql=true -Dsqlite=true -Ddriver_manager=true builddir
+ cd builddir
+ meson compile
+ - name: Test
+ run: |
+ cd c/builddir
+ meson test
Review Comment:
```suggestion
meson test -C c/build
```
##########
CONTRIBUTING.md:
##########
@@ -148,6 +148,43 @@ for details.
[cmake-prefix-path]: https://cmake.org/cmake/help/latest/variable/CMAKE_PREFIX_PATH.html
[gtest]: https://github.com/google/googletest/
+### C/C++ with Meson
+
+While CMake is the officially supported build generator, there is limited,
+experimental support for the Meson build system. Meson offers arguably better
+dependency management than CMake, with a syntax that Python developers may
+find more readable.
+
+To use Meson, start at the c directory and run:
+
+```shell
+$ meson setup builddir && cd builddir
+```
+
+For a full list of options, ``meson configure`` will bring up a pager
+with sections that you can navigate. The "Project Options" section in particular
+will show you what ADBC has to offer, and each option can be provided using
+the form ``-D_option_:_value_``. For example, to build the a debug version of
+the SQLite3 driver along with tests, you would run:
+
+```shell
+$ meson configure -Dbuildtype=debug -Dsqlite=true -Dtests=true
+```
+
+With the options set, you can then compile the project. For most dependencies,
+Meson will try to find them on your system and fall back to downloading a copy
+from its WrapDB for you:
+
+```shell
+$ meson compile
Review Comment:
```suggestion
$ meson compile -C build
```
##########
CONTRIBUTING.md:
##########
@@ -148,6 +148,43 @@ for details.
[cmake-prefix-path]: https://cmake.org/cmake/help/latest/variable/CMAKE_PREFIX_PATH.html
[gtest]: https://github.com/google/googletest/
+### C/C++ with Meson
+
+While CMake is the officially supported build generator, there is limited,
+experimental support for the Meson build system. Meson offers arguably better
+dependency management than CMake, with a syntax that Python developers may
+find more readable.
+
+To use Meson, start at the c directory and run:
+
+```shell
+$ meson setup builddir && cd builddir
+```
+
+For a full list of options, ``meson configure`` will bring up a pager
+with sections that you can navigate. The "Project Options" section in particular
+will show you what ADBC has to offer, and each option can be provided using
+the form ``-D_option_:_value_``. For example, to build the a debug version of
+the SQLite3 driver along with tests, you would run:
+
+```shell
+$ meson configure -Dbuildtype=debug -Dsqlite=true -Dtests=true
Review Comment:
```suggestion
$ meson configure -Dbuildtype=debug -Dsqlite=true -Dtests=true build
```
##########
.github/workflows/native-unix.yml:
##########
@@ -206,6 +206,38 @@ jobs:
run: |
./ci/scripts/cpp_test.sh "$(pwd)/build"
+ drivers-test-meson:
+ name: "Meson - C/C++ (Conda/${{ matrix.os }})"
+ runs-on: ${{ matrix.os }}
+ strategy:
+ matrix:
+ os: ["ubuntu-latest"]
+ steps:
+ - uses: actions/checkout@v4
+ with:
+ fetch-depth: 0
+ persist-credentials: false
+ - name: Install Dependencies
+ run: |
+ sudo apt update
+ sudo apt install -y libpq-dev ninja-build
+ - name: Setup Python
+ uses: actions/setup-python@v5
+ with:
+ python-version: '3.x'
+ - name: Install Meson via Python
+ run: pip install meson
+ - name: Build
+ run: |
+ cd c
+ meson setup -Dpostgresql=true -Dsqlite=true -Ddriver_manager=true builddir
+ cd builddir
+ meson compile
Review Comment:
How about avoiding `cd`?
```suggestion
meson setup -Dpostgresql=true -Dsqlite=true -Ddriver_manager=true c c/build
meson compile -C c/build
```
##########
c/meson.build:
##########
@@ -0,0 +1,106 @@
+# 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(
+ 'arrow-adbc',
+ 'c', 'cpp',
+ version: '0.6.0-SNAPSHOT', # TODO: can this be dynamic?
Review Comment:
We can use https://github.com/apache/arrow-adbc/blob/main/dev/release/utils-prepare.sh .
##########
CONTRIBUTING.md:
##########
@@ -148,6 +148,43 @@ for details.
[cmake-prefix-path]: https://cmake.org/cmake/help/latest/variable/CMAKE_PREFIX_PATH.html
[gtest]: https://github.com/google/googletest/
+### C/C++ with Meson
+
+While CMake is the officially supported build generator, there is limited,
+experimental support for the Meson build system. Meson offers arguably better
+dependency management than CMake, with a syntax that Python developers may
+find more readable.
+
+To use Meson, start at the c directory and run:
+
+```shell
+$ meson setup builddir && cd builddir
Review Comment:
```suggestion
$ meson setup build
```
##########
dev/release/rat_exclude_files.txt:
##########
@@ -1,6 +1,10 @@
*.json
*.Rproj
*.Rd
+c/subprojects/fmt*
+c/subprojects/gtest*
+c/subprojects/nanoarrow*
+c/subprojects/sqlite3*
Review Comment:
Can we set our license header to `c/subprojects/*.wrap` instead of excluding them?
##########
c/meson.build:
##########
@@ -0,0 +1,106 @@
+# 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(
+ 'arrow-adbc',
+ 'c', 'cpp',
+ version: '0.6.0-SNAPSHOT', # TODO: can this be dynamic?
+ license: 'Apache 2.0',
+ meson_version: '>=1.3.0',
+ default_options: [
+ 'buildtype=release',
+ 'c_std=gnu99', # TODO: gmtime_r is gnu99 - likely unintentional std
+ 'warning_level=2',
+ 'cpp_std=c++17',
+ ]
+)
+
+root_dir = include_directories('..')
+driver_dir = include_directories('driver')
+nanoarrow_dep = dependency('nanoarrow')
+fmt_dep = dependency('fmt')
+
+if get_option('tests')
+ gtest_main_dep = dependency('gtest_main')
+ # Seems to be some kind of bug with meson test
+ # gtest-port.h:1770 pthread_key_delete(key_)failed with error 22
+ # https://github.com/google/googletest/pull/4536 ???
+ gtest_dep = dependency('gtest')
+ gmock_dep = dependency('gmock')
+else
+ gtest_dep = disabler()
+ gmock_dep = disabler()
+endif
+
+if get_option('duckdb')
Review Comment:
Can we drop support for DukcDB with Meson?
I think that we don't need to maintain this because it's just for an integration test. Users don't need this and we can do it with CMake.
##########
CONTRIBUTING.md:
##########
@@ -148,6 +148,43 @@ for details.
[cmake-prefix-path]: https://cmake.org/cmake/help/latest/variable/CMAKE_PREFIX_PATH.html
[gtest]: https://github.com/google/googletest/
+### C/C++ with Meson
+
+While CMake is the officially supported build generator, there is limited,
+experimental support for the Meson build system. Meson offers arguably better
+dependency management than CMake, with a syntax that Python developers may
+find more readable.
+
+To use Meson, start at the c directory and run:
+
+```shell
+$ meson setup builddir && cd builddir
+```
+
+For a full list of options, ``meson configure`` will bring up a pager
+with sections that you can navigate. The "Project Options" section in particular
+will show you what ADBC has to offer, and each option can be provided using
+the form ``-D_option_:_value_``. For example, to build the a debug version of
+the SQLite3 driver along with tests, you would run:
+
+```shell
+$ meson configure -Dbuildtype=debug -Dsqlite=true -Dtests=true
+```
+
+With the options set, you can then compile the project. For most dependencies,
+Meson will try to find them on your system and fall back to downloading a copy
+from its WrapDB for you:
+
+```shell
+$ meson compile
+```
+
+To run the test suite, simply run:
+
+```shell
+$ meson test
Review Comment:
```suggestion
$ meson test -C 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] feat: Meson Support for ADBC [arrow-adbc]
Posted by "WillAyd (via GitHub)" <gi...@apache.org>.
WillAyd commented on code in PR #1904:
URL: https://github.com/apache/arrow-adbc/pull/1904#discussion_r1626877596
##########
dev/release/rat_exclude_files.txt:
##########
@@ -1,6 +1,10 @@
*.json
*.Rproj
*.Rd
+c/subprojects/fmt*
+c/subprojects/gtest*
+c/subprojects/nanoarrow*
+c/subprojects/sqlite3*
Review Comment:
These headers were generated by `meson wrap install ...` - happy to add that if the fact that they are automatically generated is not important
--
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] feat: Meson Support for ADBC [arrow-adbc]
Posted by "kou (via GitHub)" <gi...@apache.org>.
kou commented on code in PR #1904:
URL: https://github.com/apache/arrow-adbc/pull/1904#discussion_r1632476450
##########
c/driver/postgresql/meson.build:
##########
@@ -0,0 +1,68 @@
+# 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.
+
+libpq_dep = dependency('libpq')
+
+adbc_postgres_driver_lib = library(
+ 'adbc_driver_postgresql',
+ sources: [
+ 'connection.cc',
+ 'error.cc',
+ 'database.cc',
+ 'postgresql.cc',
+ 'result_helper.cc',
+ 'statement.cc',
+ ],
+ include_directories: [root_dir],
+ link_with: [adbc_common_lib, adbc_framework_lib],
+ dependencies: [nanoarrow_dep, fmt_dep, libpq_dep],
+)
+
+if get_option('tests')
+ postgres_tests = {
+ 'driver-postgresql': {
+ 'src_name': 'driver_postgresql',
+ 'sources': [
+ 'postgres_type_test.cc',
+ 'postgresql_test.cc',
+ ]
+ },
+ 'driver-postgresql-copy': {
+ 'src_name': 'driver_postgresql_copy',
+ 'sources': [
+ 'copy/postgres_copy_reader_test.cc',
+ 'copy/postgres_copy_writer_test.cc',
+ ]
+ },
+ }
+
+ foreach nm, conf : postgres_tests
Review Comment:
```suggestion
foreach name, conf : postgres_tests
```
##########
c/driver/postgresql/meson.build:
##########
@@ -0,0 +1,68 @@
+# 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.
+
+libpq_dep = dependency('libpq')
+
+adbc_postgres_driver_lib = library(
+ 'adbc_driver_postgresql',
+ sources: [
+ 'connection.cc',
+ 'error.cc',
+ 'database.cc',
+ 'postgresql.cc',
+ 'result_helper.cc',
+ 'statement.cc',
+ ],
+ include_directories: [root_dir],
+ link_with: [adbc_common_lib, adbc_framework_lib],
+ dependencies: [nanoarrow_dep, fmt_dep, libpq_dep],
+)
+
+if get_option('tests')
+ postgres_tests = {
+ 'driver-postgresql': {
+ 'src_name': 'driver_postgresql',
+ 'sources': [
+ 'postgres_type_test.cc',
+ 'postgresql_test.cc',
+ ]
+ },
+ 'driver-postgresql-copy': {
+ 'src_name': 'driver_postgresql_copy',
+ 'sources': [
+ 'copy/postgres_copy_reader_test.cc',
+ 'copy/postgres_copy_writer_test.cc',
+ ]
+ },
+ }
+
+ foreach nm, conf : postgres_tests
+ exc = executable(
+ 'adbc-' + nm + '-test',
Review Comment:
```suggestion
'adbc-' + name + '-test',
```
##########
CONTRIBUTING.md:
##########
@@ -148,6 +148,43 @@ for details.
[cmake-prefix-path]: https://cmake.org/cmake/help/latest/variable/CMAKE_PREFIX_PATH.html
[gtest]: https://github.com/google/googletest/
+### C/C++ with Meson
+
+While CMake is the officially supported build generator, there is limited,
+experimental support for the Meson build system. Meson offers arguably better
+dependency management than CMake, with a syntax that Python developers may
+find more readable.
+
+To use Meson, start at the c directory and run:
+
+```shell
+$ meson setup build
+```
+
+For a full list of options, ``meson configure`` will bring up a pager
+with sections that you can navigate. The "Project Options" section in particular
+will show you what ADBC has to offer, and each option can be provided using
+the form ``-D_option_:_value_``. For example, to build the a debug version of
+the SQLite3 driver along with tests, you would run:
+
+```shell
Review Comment:
```suggestion
```console
```
##########
CONTRIBUTING.md:
##########
@@ -148,6 +148,43 @@ for details.
[cmake-prefix-path]: https://cmake.org/cmake/help/latest/variable/CMAKE_PREFIX_PATH.html
[gtest]: https://github.com/google/googletest/
+### C/C++ with Meson
+
+While CMake is the officially supported build generator, there is limited,
+experimental support for the Meson build system. Meson offers arguably better
+dependency management than CMake, with a syntax that Python developers may
+find more readable.
+
+To use Meson, start at the c directory and run:
+
+```shell
Review Comment:
```suggestion
```console
```
##########
c/driver/postgresql/meson.build:
##########
@@ -0,0 +1,68 @@
+# 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.
+
+libpq_dep = dependency('libpq')
+
+adbc_postgres_driver_lib = library(
+ 'adbc_driver_postgresql',
+ sources: [
+ 'connection.cc',
+ 'error.cc',
+ 'database.cc',
+ 'postgresql.cc',
+ 'result_helper.cc',
+ 'statement.cc',
+ ],
+ include_directories: [root_dir],
+ link_with: [adbc_common_lib, adbc_framework_lib],
+ dependencies: [nanoarrow_dep, fmt_dep, libpq_dep],
+)
+
+if get_option('tests')
+ postgres_tests = {
+ 'driver-postgresql': {
+ 'src_name': 'driver_postgresql',
+ 'sources': [
+ 'postgres_type_test.cc',
+ 'postgresql_test.cc',
+ ]
+ },
+ 'driver-postgresql-copy': {
+ 'src_name': 'driver_postgresql_copy',
+ 'sources': [
+ 'copy/postgres_copy_reader_test.cc',
+ 'copy/postgres_copy_writer_test.cc',
+ ]
+ },
+ }
+
+ foreach nm, conf : postgres_tests
+ exc = executable(
+ 'adbc-' + nm + '-test',
+ sources: conf['sources'],
+ include_directories: [root_dir, driver_dir],
+ link_with: [
+ adbc_common_lib,
+ adbc_postgres_driver_lib,
+ adbc_validation_lib,
+ adbc_validation_util_lib,
+ ],
+ dependencies: [nanoarrow_dep, gtest_main_dep, gmock_dep, libpq_dep],
+ )
+ test('adbc-' + nm, exc)
Review Comment:
```suggestion
test('adbc-' + name, exc)
```
##########
CONTRIBUTING.md:
##########
@@ -148,6 +148,43 @@ for details.
[cmake-prefix-path]: https://cmake.org/cmake/help/latest/variable/CMAKE_PREFIX_PATH.html
[gtest]: https://github.com/google/googletest/
+### C/C++ with Meson
+
+While CMake is the officially supported build generator, there is limited,
+experimental support for the Meson build system. Meson offers arguably better
+dependency management than CMake, with a syntax that Python developers may
+find more readable.
+
+To use Meson, start at the c directory and run:
+
+```shell
+$ meson setup build
+```
+
+For a full list of options, ``meson configure`` will bring up a pager
+with sections that you can navigate. The "Project Options" section in particular
+will show you what ADBC has to offer, and each option can be provided using
+the form ``-D_option_:_value_``. For example, to build the a debug version of
+the SQLite3 driver along with tests, you would run:
+
+```shell
+$ meson configure -Dbuildtype=debug -Dsqlite=true -Dtests=true build
+```
+
+With the options set, you can then compile the project. For most dependencies,
+Meson will try to find them on your system and fall back to downloading a copy
+from its WrapDB for you:
+
+```shell
Review Comment:
```suggestion
```console
```
##########
c/meson.build:
##########
@@ -0,0 +1,77 @@
+# 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(
+ 'arrow-adbc',
+ 'c', 'cpp',
+ version: '1.1.0-SNAPSHOT',
+ license: 'Apache 2.0',
Review Comment:
```suggestion
license: 'Apache-2.0',
```
##########
CONTRIBUTING.md:
##########
@@ -148,6 +148,43 @@ for details.
[cmake-prefix-path]: https://cmake.org/cmake/help/latest/variable/CMAKE_PREFIX_PATH.html
[gtest]: https://github.com/google/googletest/
+### C/C++ with Meson
+
+While CMake is the officially supported build generator, there is limited,
+experimental support for the Meson build system. Meson offers arguably better
+dependency management than CMake, with a syntax that Python developers may
+find more readable.
+
+To use Meson, start at the c directory and run:
+
+```shell
+$ meson setup build
+```
+
+For a full list of options, ``meson configure`` will bring up a pager
+with sections that you can navigate. The "Project Options" section in particular
+will show you what ADBC has to offer, and each option can be provided using
+the form ``-D_option_:_value_``. For example, to build the a debug version of
+the SQLite3 driver along with tests, you would run:
+
+```shell
+$ meson configure -Dbuildtype=debug -Dsqlite=true -Dtests=true build
+```
+
+With the options set, you can then compile the project. For most dependencies,
+Meson will try to find them on your system and fall back to downloading a copy
+from its WrapDB for you:
+
+```shell
+$ meson compile -C build
+```
+
+To run the test suite, simply run:
+
+```shell
Review Comment:
```suggestion
```console
```
--
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] feat: Meson Support for ADBC [arrow-adbc]
Posted by "WillAyd (via GitHub)" <gi...@apache.org>.
WillAyd commented on code in PR #1904:
URL: https://github.com/apache/arrow-adbc/pull/1904#discussion_r1625083196
##########
c/meson.build:
##########
Review Comment:
Left some TODOs in this file on things that were tripping me up. I don't know enough about Go to make that work, though the issue I linked to might have some workarounds.
DuckDB in theory could be wrangled well as a subproject, but the CMake <> Meson bridge is a bit wonky and the generated includes create a conflict between the adbc.h file in this repository versus the adbc.h file that DuckDB creates
--
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] feat: Meson Support for ADBC [arrow-adbc]
Posted by "kou (via GitHub)" <gi...@apache.org>.
kou commented on code in PR #1904:
URL: https://github.com/apache/arrow-adbc/pull/1904#discussion_r1632481166
##########
CONTRIBUTING.md:
##########
@@ -148,6 +148,43 @@ for details.
[cmake-prefix-path]: https://cmake.org/cmake/help/latest/variable/CMAKE_PREFIX_PATH.html
[gtest]: https://github.com/google/googletest/
+### C/C++ with Meson
+
+While CMake is the officially supported build generator, there is limited,
+experimental support for the Meson build system. Meson offers arguably better
+dependency management than CMake, with a syntax that Python developers may
+find more readable.
+
+To use Meson, start at the c directory and run:
+
+```shell
Review Comment:
Oh. Could you use 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] feat: Meson Support for ADBC [arrow-adbc]
Posted by "WillAyd (via GitHub)" <gi...@apache.org>.
WillAyd commented on code in PR #1904:
URL: https://github.com/apache/arrow-adbc/pull/1904#discussion_r1632496869
##########
CONTRIBUTING.md:
##########
@@ -148,6 +148,43 @@ for details.
[cmake-prefix-path]: https://cmake.org/cmake/help/latest/variable/CMAKE_PREFIX_PATH.html
[gtest]: https://github.com/google/googletest/
+### C/C++ with Meson
+
+While CMake is the officially supported build generator, there is limited,
+experimental support for the Meson build system. Meson offers arguably better
+dependency management than CMake, with a syntax that Python developers may
+find more readable.
+
+To use Meson, start at the c directory and run:
+
+```shell
Review Comment:
Sure - will open after this goes in. For now I left this file alone, but made the updates you suggested to other files
--
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] feat: Meson Support for ADBC [arrow-adbc]
Posted by "WillAyd (via GitHub)" <gi...@apache.org>.
WillAyd commented on code in PR #1904:
URL: https://github.com/apache/arrow-adbc/pull/1904#discussion_r1632479122
##########
CONTRIBUTING.md:
##########
@@ -148,6 +148,43 @@ for details.
[cmake-prefix-path]: https://cmake.org/cmake/help/latest/variable/CMAKE_PREFIX_PATH.html
[gtest]: https://github.com/google/googletest/
+### C/C++ with Meson
+
+While CMake is the officially supported build generator, there is limited,
+experimental support for the Meson build system. Meson offers arguably better
+dependency management than CMake, with a syntax that Python developers may
+find more readable.
+
+To use Meson, start at the c directory and run:
+
+```shell
Review Comment:
No problem making this change but all of the other entries in the CONTRIBUTING.md guide currently use `shell` - do you want me to update all of them as part of this or leave that to 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] feat: Meson Support for ADBC [arrow-adbc]
Posted by "WillAyd (via GitHub)" <gi...@apache.org>.
WillAyd commented on code in PR #1904:
URL: https://github.com/apache/arrow-adbc/pull/1904#discussion_r1626928132
##########
dev/release/rat_exclude_files.txt:
##########
@@ -1,6 +1,10 @@
*.json
*.Rproj
*.Rd
+c/subprojects/fmt*
+c/subprojects/gtest*
+c/subprojects/nanoarrow*
+c/subprojects/sqlite3*
Review Comment:
No problem. I made these wildcards to prevent anything generated by the subprojects themselves being included, but those should never be committed anyway. Changed to the more explicit form you have suggested
--
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] feat: Meson Support for ADBC [arrow-adbc]
Posted by "lidavidm (via GitHub)" <gi...@apache.org>.
lidavidm commented on PR #1904:
URL: https://github.com/apache/arrow-adbc/pull/1904#issuecomment-2148567999
@kou I think you're much more familiar with Meson, any thoughts?
--
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] feat: Meson Support for ADBC [arrow-adbc]
Posted by "WillAyd (via GitHub)" <gi...@apache.org>.
WillAyd commented on code in PR #1904:
URL: https://github.com/apache/arrow-adbc/pull/1904#discussion_r1626684650
##########
c/meson.build:
##########
@@ -0,0 +1,106 @@
+# 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(
+ 'arrow-adbc',
+ 'c', 'cpp',
+ version: '0.6.0-SNAPSHOT', # TODO: can this be dynamic?
+ license: 'Apache 2.0',
+ meson_version: '>=1.3.0',
+ default_options: [
+ 'buildtype=release',
+ 'c_std=gnu99', # TODO: gmtime_r is gnu99 - likely unintentional std
+ 'warning_level=2',
+ 'cpp_std=c++17',
+ ]
+)
+
+root_dir = include_directories('..')
+driver_dir = include_directories('driver')
+nanoarrow_dep = dependency('nanoarrow')
+fmt_dep = dependency('fmt')
+
+if get_option('tests')
+ gtest_main_dep = dependency('gtest_main')
+ # Seems to be some kind of bug with meson test
+ # gtest-port.h:1770 pthread_key_delete(key_)failed with error 22
+ # https://github.com/google/googletest/pull/4536 ???
+ gtest_dep = dependency('gtest')
+ gmock_dep = dependency('gmock')
+else
+ gtest_dep = disabler()
+ gmock_dep = disabler()
+endif
+
+if get_option('duckdb')
+ cmake = import('cmake')
+ opt_var = cmake.subproject_options()
+ # Doesn't appear duckdb properly builds PIC shared libraries as a subproject
+ # https://github.com/mesonbuild/meson/issues/10764
+ # Adding these args is a dirty hack
+ compiler_id = meson.get_compiler('c').get_id()
+ if compiler_id == 'gcc' or compiler_id == 'clang'
+ opt_var.append_compile_args('c', '-fPIC')
+ opt_var.append_compile_args('cpp', '-fPIC')
+ endif
+ duckdb_proj = cmake.subproject('duckdb', options: opt_var)
+ duckdb_dep = duckdb_proj.dependency('duckdb')
+else
+ duckdb_dep = disabler()
+endif
+
+needs_driver_manager = get_option('driver_manager') \
+ or get_option('duckdb') \
+ or get_option('tests')
+
+if needs_driver_manager
+ subdir('driver_manager')
+endif
+
+subdir('driver/common')
+subdir('driver/framework')
+
+if get_option('tests')
+ subdir('validation')
+endif
+
+if get_option('flightsql')
+ subdir('driver/flightsql')
+endif
+
+if get_option('postgresql')
+ subdir('driver/postgresql')
+endif
+
+if get_option('sqlite')
+ subdir('driver/sqlite')
+endif
+
+if get_option('snowflake')
+ subdir('driver/snowflake')
+endif
+
+if get_option('duckdb')
+ # TODO: currently broken because #include <adbc.h> conflicts between duckdb /
+ # arrow-adbc includes
+ subdir('integration/duckdb')
+endif
+
+if get_option('python')
Review Comment:
I put this in for consistency with our current CMake implementation, but I think in the long run it would be easier to use meson-python and have it take care of dependencies automatically via pip install, rather than trying to toggle via a build generator option
--
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] feat: Meson Support for ADBC [arrow-adbc]
Posted by "WillAyd (via GitHub)" <gi...@apache.org>.
WillAyd commented on code in PR #1904:
URL: https://github.com/apache/arrow-adbc/pull/1904#discussion_r1626684170
##########
c/meson.build:
##########
@@ -0,0 +1,106 @@
+# 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(
+ 'arrow-adbc',
+ 'c', 'cpp',
+ version: '0.6.0-SNAPSHOT', # TODO: can this be dynamic?
+ license: 'Apache 2.0',
+ meson_version: '>=1.3.0',
+ default_options: [
+ 'buildtype=release',
+ 'c_std=gnu99', # TODO: gmtime_r is gnu99 - likely unintentional std
+ 'warning_level=2',
+ 'cpp_std=c++17',
+ ]
+)
+
+root_dir = include_directories('..')
+driver_dir = include_directories('driver')
+nanoarrow_dep = dependency('nanoarrow')
+fmt_dep = dependency('fmt')
+
+if get_option('tests')
+ gtest_main_dep = dependency('gtest_main')
+ # Seems to be some kind of bug with meson test
+ # gtest-port.h:1770 pthread_key_delete(key_)failed with error 22
+ # https://github.com/google/googletest/pull/4536 ???
+ gtest_dep = dependency('gtest')
+ gmock_dep = dependency('gmock')
+else
+ gtest_dep = disabler()
+ gmock_dep = disabler()
+endif
+
+if get_option('duckdb')
Review Comment:
This might be tough to get working in its current state - probably will defer to fully fixing for a later 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] feat: Meson Support for ADBC [arrow-adbc]
Posted by "kou (via GitHub)" <gi...@apache.org>.
kou commented on code in PR #1904:
URL: https://github.com/apache/arrow-adbc/pull/1904#discussion_r1626921213
##########
dev/release/rat_exclude_files.txt:
##########
@@ -1,6 +1,10 @@
*.json
*.Rproj
*.Rd
+c/subprojects/fmt*
+c/subprojects/gtest*
+c/subprojects/nanoarrow*
+c/subprojects/sqlite3*
Review Comment:
OK. Let's exclude `*.wrap`. Can we narrow exclude targets as much as possible such as using `fmt.wrap` instead of `fmt.*`?
--
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