search

llloret / osmid

osmid is a tool to bridge MIDI and OSC. It is currently in use in Sonic Pi
Other
72 stars 12 forks source link

allow devendoring of spdlog #60

Open dvzrv opened 4 years ago

dvzrv commented 4 years ago

Hi! I'm maintaining osmid for Arch Linux.

As on most Linux distributions, we already have spdlog (and in a much newer version) it would be great to allow proper devendoring of the library (e.g. via CMake option).

Vendoring (aka bundling) of libraries (especially very old ones) is never a good idea. Fedora has a good writeup as to why that is.

The spdlog library offers both pkgconfig and cmake integration out-of-the-box, so discovery and integration of a system version should be trivial.

llloret commented 4 years ago

Ok, makes sense, but do you have any advice about what to do for Windows, then?

dvzrv commented 4 years ago

Yeah, Windows is always a bit of a special case, I guess. ;-)

My proposal is to allow devendoring, not to enforce it. For environments such as Windows it could still remain vendored. However, other operating systems such as Linux have different requirements.

If allowing to use a system version of spdlog is done via a CMake option (e.g. USE_SYSTEM_SPDLOG), it should be fairly trivial: just default to OFF). If it is ON, cmake discovers and uses the system version, if it is OFF the vendored version is used. If the target include paths are set correctly there should not even be any difference for the code (and all can be set to global includes).

Closing, I guess it would also be great to upgrade the vendored version (to remain in sync with its upstream).

llloret commented 4 years ago

@dvzrv , ok, I have put the change in the branch spdlog_from_distro. Can you please have a look and let me know if that is what you were expecting?

I have also updated the bundled spdlog in that branch to make it compatible with the one I see packaged for my Linux distribution, so that I can use the same source code in the application.

If this is what you were looking for, I'll merge into master and tag a new release.

dvzrv commented 4 years ago

Argh, sorry... I forgot about this ticket! I'll look into this tomorrow for sure as I have to update the package... :)

dvzrv commented 4 years ago

Okay, I have commented on a small issue with the fix.

Unfortunately we are already at spdlog 1.7.0 and building against it does not work: osmid-0.8.0-devendor_spdlog.txt