feat: Meson Support for ADBC #1904

WillAyd · 2024-06-03T21:41:14Z

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

Fixes #1941.

WillAyd · 2024-06-03T21:43:29Z

c/meson.build

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

WillAyd · 2024-06-04T22:42:04Z

c/meson.build

+    gmock_dep = disabler()
+endif
+
+if get_option('duckdb')


This might be tough to get working in its current state - probably will defer to fully fixing for a later PR

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.

WillAyd · 2024-06-04T22:42:58Z

c/meson.build

+  subdir('integration/duckdb')
+endif
+
+if get_option('python')


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

lidavidm · 2024-06-04T23:21:13Z

@kou I think you're much more familiar with Meson, any thoughts?

kou

I think that Meson support is OK but we should keep maintenance cost as small as possible. Because our main build system is CMake. In general, maintaining multiple build systems are time consuming.

So I think that we should support only simple and needed build features for Meson. Other complex build features are implemented only with CMake.

kou · 2024-06-05T02:33:28Z

.github/workflows/native-unix.yml

+          cd c
+          meson setup -Dpostgresql=true -Dsqlite=true -Ddriver_manager=true builddir
+          cd builddir
+          meson compile


How about avoiding cd?

Suggested change

cd c

meson setup -Dpostgresql=true -Dsqlite=true -Ddriver_manager=true builddir

cd builddir

meson compile

meson setup -Dpostgresql=true -Dsqlite=true -Ddriver_manager=true c c/build

meson compile -C c/build

kou · 2024-06-05T02:33:51Z

.github/workflows/native-unix.yml

+          cd c/builddir
+          meson test


Suggested change

cd c/builddir

meson test

meson test -C c/build

kou · 2024-06-05T02:34:20Z

.gitignore

+c/subprojects/*
+!c/subprojects/*.wrap


Suggested change

c/subprojects/*

!c/subprojects/*.wrap

/c/subprojects/*

!/c/subprojects/*.wrap

kou · 2024-06-05T02:34:40Z

CONTRIBUTING.md

+To use Meson, start at the c directory and run:
+
+```shell
+$ meson setup builddir && cd builddir


Suggested change

$ meson setup builddir && cd builddir

$ meson setup build

kou · 2024-06-05T02:35:25Z

CONTRIBUTING.md

+the SQLite3 driver along with tests, you would run:
+
+```shell
+$ meson configure -Dbuildtype=debug -Dsqlite=true -Dtests=true


Suggested change

$ meson configure -Dbuildtype=debug -Dsqlite=true -Dtests=true

$ meson configure -Dbuildtype=debug -Dsqlite=true -Dtests=true build

kou · 2024-06-05T02:35:32Z

CONTRIBUTING.md

+from its WrapDB for you:
+
+```shell
+$ meson compile


Suggested change

$ meson compile

$ meson compile -C build

kou · 2024-06-05T02:35:38Z

CONTRIBUTING.md

+To run the test suite, simply run:
+
+```shell
+$ meson test


Suggested change

$ meson test

$ meson test -C build

kou · 2024-06-05T03:13:27Z

c/meson.build

+project(
+    'arrow-adbc',
+    'c', 'cpp',
+    version: '0.6.0-SNAPSHOT',  # TODO: can this be dynamic?


We can use https://github.com/apache/arrow-adbc/blob/main/dev/release/utils-prepare.sh .

kou · 2024-06-05T03:19:20Z

dev/release/rat_exclude_files.txt

+c/subprojects/fmt*
+c/subprojects/gtest*
+c/subprojects/nanoarrow*
+c/subprojects/sqlite3*


Can we set our license header to c/subprojects/*.wrap instead of excluding them?

These headers were generated by meson wrap install ... - happy to add that if the fact that they are automatically generated is not important

OK. Let's exclude *.wrap. Can we narrow exclude targets as much as possible such as using fmt.wrap instead of fmt.*?

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

kou · 2024-06-05T03:29:02Z

c/meson.build

+    gmock_dep = disabler()
+endif
+
+if get_option('duckdb')


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.

kou

+1

kou · 2024-06-10T01:06:05Z

CONTRIBUTING.md

+
+To use Meson, start at the c directory and run:
+
+```shell


Suggested change

```shell

```console

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?

Oh. Could you use a separate PR?

Sure - will open after this goes in. For now I left this file alone, but made the updates you suggested to other files

kou · 2024-06-10T01:06:11Z

CONTRIBUTING.md

+the form ``-D_option_:_value_``. For example, to build the a debug version of
+the SQLite3 driver along with tests, you would run:
+
+```shell


Suggested change

```shell

```console

kou · 2024-06-10T01:06:23Z

CONTRIBUTING.md

+Meson will try to find them on your system and fall back to downloading a copy
+from its WrapDB for you:
+
+```shell


Suggested change

```shell

```console

kou · 2024-06-10T01:06:28Z

CONTRIBUTING.md

+
+To run the test suite, simply run:
+
+```shell


Suggested change

```shell

```console

kou · 2024-06-10T01:08:00Z

c/driver/postgresql/meson.build

+        },
+    }
+
+    foreach nm, conf : postgres_tests


Suggested change

foreach nm, conf : postgres_tests

foreach name, conf : postgres_tests

kou · 2024-06-10T01:08:05Z

c/driver/postgresql/meson.build

+
+    foreach nm, conf : postgres_tests
+        exc = executable(
+            'adbc-' + nm + '-test',


Suggested change

'adbc-' + nm + '-test',

'adbc-' + name + '-test',

kou · 2024-06-10T01:08:12Z

c/driver/postgresql/meson.build

+            ],
+            dependencies: [nanoarrow_dep, gtest_main_dep, gmock_dep, libpq_dep],
+        )
+        test('adbc-' + nm, exc)


Suggested change

test('adbc-' + nm, exc)

test('adbc-' + name, exc)

kou · 2024-06-10T01:09:26Z

c/meson.build

+    'arrow-adbc',
+    'c', 'cpp',
+    version: '1.1.0-SNAPSHOT',
+    license: 'Apache 2.0',


Suggested change

license: 'Apache 2.0',

license: 'Apache-2.0',

lidavidm · 2024-06-25T06:24:49Z

@kou any other feedback here?

kou · 2024-06-25T21:59:00Z

No!
Let's merge this.

kou · 2024-06-25T21:59:54Z

Do we need an associated issue before we merge this?

lidavidm · 2024-06-25T23:21:58Z

I attached #1941

feat: Meson Support for ADBC

7654643

WillAyd commented Jun 3, 2024

View reviewed changes

WillAyd added 11 commits June 3, 2024 16:41

fixes

aeccd96

Limited Go support

01a14a7

Exclude nanoarrow check

ff6b9b0

Add Meson to CI

425de68

Install Meson to CI

fe6333e

Add cd

a15b522

cd builddir

8205186

libpq install

666fb65

Install on ubuntu

0d46d2c

Install newer Meson

8b6a8d2

change test directory

f0fbb32

WillAyd commented Jun 4, 2024

View reviewed changes

Go platform difference smoothing

4e95dda

kou reviewed Jun 5, 2024

View reviewed changes

kou feedback

3a4006f

WillAyd marked this pull request as ready for review June 5, 2024 04:07

WillAyd requested review from lidavidm and zeroshade as code owners June 5, 2024 04:07

More explicit rat exclusion

3646ade

github-actions bot added this to the ADBC Libraries 13 milestone Jun 5, 2024

lidavidm approved these changes Jun 10, 2024

View reviewed changes

kou approved these changes Jun 10, 2024

View reviewed changes

kou feedback

8ed7c04

lidavidm merged commit 453a9c3 into apache:main Jun 25, 2024
65 checks passed

WillAyd deleted the meson-support branch June 26, 2024 15:45

	$ meson configure -Dbuildtype=debug -Dsqlite=true -Dtests=true
	$ meson configure -Dbuildtype=debug -Dsqlite=true -Dtests=true build

	foreach nm, conf : postgres_tests
	foreach name, conf : postgres_tests

feat: Meson Support for ADBC #1904

feat: Meson Support for ADBC #1904

Conversation

WillAyd commented Jun 3, 2024 • edited by lidavidm Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

lidavidm commented Jun 4, 2024

kou left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

kou left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

lidavidm commented Jun 25, 2024

kou commented Jun 25, 2024

kou commented Jun 25, 2024

lidavidm commented Jun 25, 2024

WillAyd commented Jun 3, 2024 •

edited by lidavidm

Loading