Meson support #413

WillAyd · 2024-04-01T15:19:55Z

Not sure if there is a desire for supporting another build backend, but figured I would share in case.

Meson seems a bit simpler than the existing CMake configuration, and has the advantage of:

Installing dependencies via the wrap database easily
Potentially making nanoarrow installable via the WrapDB

Point 2 is not covered here, but might be a nice future enhancement for downstream Python libraries like NumPy and pandas that use Meson as their build systems

codecov-commenter · 2024-04-01T15:28:40Z

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 88.76%. Comparing base (a9ef745) to head (bfaea57).
Report is 1 commits behind head on main.

Additional details and impacted files

@@           Coverage Diff           @@
##             main     #413   +/-   ##
=======================================
  Coverage   88.76%   88.76%           
=======================================
  Files          82       82           
  Lines       14633    14633           
=======================================
  Hits        12989    12989           
  Misses       1644     1644

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

paleolimbot

Thank you for opening this!

I'm definitely open to this for nanoarrow as long as the options that are exposed are tested and there is an example somewhere of how to use nanoarrow as a meson dependency. Off the top of my head, a script like ci/scripts/meson-build.sh might be a low-effort way to do this, and we could maybe add it as a weekly job (like release verification)?

WillAyd · 2024-04-02T11:35:27Z

Sounds good - I just copied over the minimal example and switched it to Meson. It is still a little different from what end users would do long term (i.e. would be nice to have nanoarrow in the WrapDB) but I think the gist of it is there

ci/scripts/build-with-meson.sh

paleolimbot

would be nice to have nanoarrow in the WrapDB)

Perhaps after 0.5.0 (in a month or so) it could be submitted? No pressure, but if you'd be willing to take that on I'd be happy to support anything else that needs to happen.

ci/scripts/build-with-meson.sh

paleolimbot · 2024-04-02T15:21:44Z

examples/meson-minimal/src/app.c

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).

Can you point me to which example you are talking about? I copied the source code for this over from cmake-minimal

paleolimbot · 2024-04-02T15:23:29Z

examples/meson-minimal/subprojects/nanoarrow.wrap

+# KIND, either express or implied.  See the License for the
+# specific language governing permissions and limitations
+# under the License.
+


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?)

meson.build

paleolimbot · 2024-04-02T15:26:44Z

src/nanoarrow/meson.build

+# TODO: CMake uses a regex to extract major/minor/patch from this string
+version = '0.5.0-SNAPSHOT'


This can be hard-coded as long as it's in the version-bumping script (linked above)

I ended up replacing this in the latest version to pull from the project configuration. I don't believe meson supports regular expressions so its a bit more convoluted than the CMake version to split that into major / minor / patch, but I think should work

paleolimbot · 2024-04-02T15:29:03Z

src/nanoarrow/meson.build

+  # 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?


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.

src/nanoarrow/meson.build

paleolimbot · 2024-04-02T15:36:15Z

.github/workflows/meson-build.yaml

+    paths:
+      - '.github/workflows/meson-build.yaml'
+      - 'ci/scripts/build-with-meson.sh'


Probably this should also run when any of the config or options or build files are modified?

WillAyd · 2024-04-02T15:55:53Z

Yea happy to do so

raulcd · 2024-04-02T15:58:41Z

dev/benchmarks/c/meson.build

+incdir = '../../../src'
+incdir2 = '../../../src/nanoarrow'


thanks for the PR, I am not an expert on meson but this feels odd. Isn't there a way to not hardcode those directory paths?

Nice catch - I set that up before I created the nanoarrow_dep target, but can switch over to that

paleolimbot

I took a run through the READMEs and found a couple of things! Meson is cool!

paleolimbot · 2024-04-03T12:44:15Z

.github/workflows/meson-build.yaml

+      - 'meson.build'
+      - 'meson.options'
+      - 'src/nanoarrow/meson.build'
+      - 'dev/benchmarks/meson.build'
+      - 'dev/benchmarks/c/meson.build'


Suggested change

- 'meson.build'

- 'meson.options'

- 'src/nanoarrow/meson.build'

- 'dev/benchmarks/meson.build'

- 'dev/benchmarks/c/meson.build'

- '**meson.build'

- '**meson.options'

There should only ever be one meson.options file at the project root so I didn't add the wildcard for that. But nice idea on the meson.build files

.github/workflows/meson-build.yaml

paleolimbot · 2024-04-03T12:47:29Z

examples/meson-minimal/README.md

+  under the License.
+-->
+
+# Minimal CMake Example


Suggested change

# Minimal CMake Example

# Minimal Meson Example

paleolimbot · 2024-04-03T12:51:04Z

examples/meson-minimal/src/app.c

I still think that the minimal example could be smaller here:

arrow-nanoarrow/examples/cmake-scenarios/src/app.cpp

Lines 18 to 26 in 9935713

#include "nanoarrow/nanoarrow.h"

#include "nanoarrow/nanoarrow.hpp"

int main(int argc, char* argv[]) {

nanoarrow::UniqueSchema schema;

NANOARROW_RETURN_NOT_OK(ArrowSchemaInitFromType(schema.get(), NANOARROW_TYPE_INT32));

printf("Schema format for int32 is '%s'", schema->format);

return EXIT_SUCCESS;

}

README.md

paleolimbot · 2024-04-03T13:38:21Z

README.md

+Upon a successful build you can execute the test suite and benchmarks with the following commands:
+
+```sh
+meson test   # default test run


Suggested change

meson test # default test run

meson test nanoarrow: # default test run

paleolimbot · 2024-04-03T13:39:32Z

README.md

+```sh
+meson test   # default test run
+meson test --wrap valgrind  # run tests under valgrind
+meson test --benchmark # run benchmarks


Suggested change

meson test --benchmark # run benchmarks

meson test --benchmark --verbose # run benchmarks

paleolimbot · 2024-04-03T13:43:12Z

examples/meson-minimal/README.md

+resulting length (or any error encountered whilst building the array).
+
+```bash
+./example_cmake_minimal_app


Suggested change

./example_cmake_minimal_app

./example_meson_minimal_app

WillAyd · 2024-04-08T17:29:59Z

I believe I addressed all feedback but let me know if anything is outstanding

paleolimbot

Sorry...I missed that notification! Thank you!

Not sure if there is a desire for supporting another build backend, but figured I would share in case. Meson seems a bit simpler than the existing CMake configuration, and has the advantage of: 1. Installing dependencies via the wrap database easily 2. Potentially making nanoarrow installable via the [WrapDB](https://mesonbuild.com/Adding-new-projects-to-wrapdb.html) Point 2 is not covered here, but might be a nice future enhancement for downstream Python libraries like NumPy and pandas that use Meson as their build systems

WillAyd added 4 commits April 1, 2024 10:43

Working meson files

49a8e4b

More improvements

5379560

Documentation and fixups

8bf072f

Merge remote-tracking branch 'upstream/main' into meson-support

715353b

Add licenses

4e3cb29

paleolimbot reviewed Apr 1, 2024

View reviewed changes

WillAyd added 4 commits April 1, 2024 18:36

Add job to CI

72c95a4

Subproject / wraps support

5d9647c

Working subproject example

32b2ffb

Missing license

abf593c

WillAyd marked this pull request as ready for review April 2, 2024 01:19

WillAyd added 15 commits April 2, 2024 07:37

Update Meson build action

f72b3a0

Add ninja

b1d233f

ninja -> ninja-build

9e46886

Install Arrow

4b8b19c

meson_options -> meson.options

c9117b5

Try pkg-config

54cae53

PKG_CONFIG_PATH

6fe187a

debug

6fdab80

Try updated PATHs

6091be7

Add /pkgconfig

2847573

pkg-config debug

27d8523

try CLI arg

1e0d66c

Remove static/shared CLI flag and use pkg-config

58044bf

Separate tests / benchmarks

2703ced

typo

6383a29

WillAyd commented Apr 2, 2024

View reviewed changes

ci/scripts/build-with-meson.sh Show resolved Hide resolved

ci/scripts/build-with-meson.sh Show resolved Hide resolved

paleolimbot reviewed Apr 2, 2024

View reviewed changes

raulcd reviewed Apr 2, 2024

View reviewed changes

WillAyd added 2 commits April 2, 2024 12:44

dewey feedback

c763407

raulcd feedback

0005991

paleolimbot reviewed Apr 3, 2024

View reviewed changes

WillAyd added 2 commits April 3, 2024 10:36

more feedback

bfaea57

Add cache step

cb6f5bc

paleolimbot approved these changes Apr 8, 2024

View reviewed changes

paleolimbot merged commit 22eab6b into apache:main Apr 8, 2024
33 checks passed

WillAyd deleted the meson-support branch April 8, 2024 18:45

paleolimbot added this to the nanoarrow 0.5.0 milestone May 22, 2024

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Meson support #413

Meson support #413

WillAyd commented Apr 1, 2024

codecov-commenter commented Apr 1, 2024 •

edited

Loading

paleolimbot left a comment

WillAyd commented Apr 2, 2024

paleolimbot left a comment

paleolimbot Apr 2, 2024

WillAyd Apr 2, 2024

paleolimbot Apr 2, 2024

paleolimbot Apr 2, 2024

WillAyd Apr 2, 2024

paleolimbot Apr 2, 2024

paleolimbot Apr 2, 2024

WillAyd commented Apr 2, 2024

raulcd Apr 2, 2024

WillAyd Apr 2, 2024

paleolimbot left a comment

paleolimbot Apr 3, 2024

WillAyd Apr 3, 2024

paleolimbot Apr 3, 2024

paleolimbot Apr 3, 2024

paleolimbot Apr 3, 2024

paleolimbot Apr 3, 2024

paleolimbot Apr 3, 2024

WillAyd commented Apr 8, 2024

paleolimbot left a comment

		# TODO: CMake uses a regex to extract major/minor/patch from this string
		version = '0.5.0-SNAPSHOT'

	#include "nanoarrow/nanoarrow.h"
	#include "nanoarrow/nanoarrow.hpp"

	int main(int argc, char* argv[]) {
	nanoarrow::UniqueSchema schema;
	NANOARROW_RETURN_NOT_OK(ArrowSchemaInitFromType(schema.get(), NANOARROW_TYPE_INT32));
	printf("Schema format for int32 is '%s'", schema->format);
	return EXIT_SUCCESS;
	}

	meson test # default test run
	meson test nanoarrow: # default test run

	meson test --benchmark # run benchmarks
	meson test --benchmark --verbose # run benchmarks

		incdir = '../../../src'
		incdir2 = '../../../src/nanoarrow'

Meson support #413

Meson support #413

Conversation

WillAyd commented Apr 1, 2024

codecov-commenter commented Apr 1, 2024 • edited Loading

Codecov Report

paleolimbot left a comment

Choose a reason for hiding this comment

WillAyd commented Apr 2, 2024

paleolimbot 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

WillAyd commented Apr 2, 2024

Choose a reason for hiding this comment

Choose a reason for hiding this comment

paleolimbot 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

WillAyd commented Apr 8, 2024

paleolimbot left a comment

Choose a reason for hiding this comment

codecov-commenter commented Apr 1, 2024 •

edited

Loading