apache / arrow-nanoarrow

Helpers for Arrow C Data & Arrow C Stream interfaces
https://arrow.apache.org/nanoarrow
Apache License 2.0
169 stars 35 forks source link

feat: Meson build system for nanoarrow-ipc extension #483

Closed WillAyd closed 3 months ago

WillAyd commented 4 months ago

Meson's handling of subprojects is way easier I think than CMake's. If down the road we wanted to replace CMake with Meson there is a ton that we could simplify within these extensions, and maybe could even consider them as separate projects

paleolimbot commented 4 months ago

Nice!

I've been wondering if the IPC and Device "extensions" should move into the main tree (e.g., https://github.com/apache/arrow-nanoarrow/pull/481 )...they started as separate projects but a few things like the "testing" module make it a little awkward to keep them separate. Any thoughts on that?

WillAyd commented 4 months ago

I haven't used these in particular so I'm just kind of guessing, but it feels like it would make sense to combine them. I think for consumers having to potentially install three different libraries for nanoarrow instead of just one library that you can choose the dependencies out of is overkill. And as you've mentioned already, there appears to be a ton of overlap between these projects' dependencies

WillAyd commented 4 months ago

Let's just wait to merge this until after then - there is no rush for this and I think waiting will make that consolidation easier

WillAyd commented 3 months ago

Interesting to see the valgrind error in CI right now. I assume this is also why the windows build is failing, although surprised our CMake setup didn't catch it

https://github.com/apache/arrow-nanoarrow/actions/runs/9455933515/job/26046780100?pr=483#step:7:338

WillAyd commented 3 months ago

Seems like the error has to do with the fields being sent to ArrowErrorSet in this function:

https://github.com/apache/arrow-nanoarrow/blob/f443e46163ded43ddb5e024c1a2f1fddd3b43c3f/src/nanoarrow/array.c#L1247

Not familiar with the run end encoded implementation but I think we are missing something with how fields are being initialized and accessed through that code block