If helpful for local debugging you might want to try setting this up through Meson, as that work is a lot simpler than what CMake requires. I think we used to even have this set up back in #531 but reverted due to some issues

Love the individual commit names - really takes you along for the ride as a reader. Hope all is well!

@assignUser I hate to bug you again, but there's quite a big change to the CMake that would benefit from your 👀 if you have the bandwidth!

I have some thoughts about target naming and such but I need to take some more time and think about it and look some things up.

Let me know here or elsewhere when you have time to circle back!

One thing is that I see a bunch of repeating stuff that should probably be refactored in a function, but that's true in general (see L436 onwards) so that could be done as a follow up.

Great point! I've opened an issue to track this one: #726 .

I think you should change nanoarrow explicitly to nanoarrow_static to be consistent and then add a nanoarrow::nanoarrow alias to it (I assume static will be the default? Or you could base that on BUILD_SHARED_LIBS like this

if(BUILD_SHARED_LIBS)
  add_library(nanoarrow::nanoarrow ALIAS nanoarrow_shared)
else()
  add_library(nanoarrow::nanoarrow ALIAS nanoarrow_static)
endif()

This is mostly relevant when nanoarrow is built as a subproject. You could also add an option that allows toggling the building of either version (gflags does that). That way I don't have to compile everything twice if I only care about static.

For packaging the shared bit could be made a component that also changes which target gets the main alias. (again something gflags does find_package(gflags COMPONENTS shared)

nit: You should be able to create an INTERFACE target for the sources and use that instead of variables: https://cmake.org/cmake/help/latest/prop_tgt/INTERFACE_SOURCES.html

I haven't used an interface target but it's a nice solution to avoid raw variables (which I am always happy about...)

I think you should change nanoarrow explicitly to nanoarrow_static to be consistent

Done! I do like this better. I also added configurable test linkage for better feedback if this breaks for some reason.

then add a nanoarrow::nanoarrow alias to it

This is also sort of a breaking change (target_link_libraries(... nanoarrow::nanoarrow) vs target_link_libraries(... nanoarrow) and I think explicit nanoarrow_static is better (possibly at the expense of being slightly confusing to somebody who doesn't know what static linking is, like me threeish years ago 🙂 ). I'm personally OK with the breaking change here but could definitely be convinced otherwise.

For packaging the shared bit could be made a component that also changes which target gets the main alias.

I'll leave this one for now since it doesn't affect the interface and because our compile times are very low but it's a great point that the average we-use-nanoarrow-as-a-subproject only needs one of the two.

nit: You should be able to create an INTERFACE target for the sources and use that instead of variables

I'll add a note about this one in #726 since it's in the category of nice-to-have CMake improvements that I don't quite understand yet 🙂

Thanks for all the work!

For my understanding, for downstream libraries that link to nanoarrow, this will mean to be used like this:

target_link_libraries(main PRIVATE $<IF:$<TARGET_EXISTS:nanoarrow::nanoarrow_shared>,nanoarrow::nanoarrow_shared,nanoarrow::nanoarrow_static>)

and there's no generic target that takes whatever has been built?

target_link_libraries(main PRIVATE nanoarrow::nanoarrow)

I'm asking as I never had the need to build static AND shared libraries side-by-side but had the need to build either one or the other depending on the target platform (e.g. shared for linux, static for ios) and for these cases the short version is nice in terms of readability.

then add a nanoarrow::nanoarrow alias to it

Ok, I added this + tests to make sure it works in both the fetch and installed cases. You and @m-kuhn are right that this is sort of annoying if you want the value of BUILD_SHARED_LIBS to do the work for you.

For my understanding, for downstream libraries that link to nanoarrow, this will mean to be used like this:

It would probably be more like the if(BUILD_SHARED_LIBS) ... target_link_libraries(... nanoarrow::nanoarrow_shared) like Jacob demoed above...both targets are always available but you'd have to decide which one you want. There are some cases where you want to namespace the symbols + statically link even though you yourself are building a shared library. In any case, linking to nanoarrow::nanoarrow should do the same thing that it did before this PR.

Thank you all for the reviews here!