marzer / tomlplusplus

Header-only TOML config file parser and serializer for C++17.
https://marzer.github.io/tomlplusplus/
MIT License
1.53k stars 146 forks source link

Support options in Conan #227

Closed xoorath closed 3 months ago

xoorath commented 3 months ago

Hey,

It would be nice to be able to set the configuration options from conan as settings.

Ping @prince-chrismc for your thoughts as well since you're credited with the conancenter addition.

Cheers,

marzer commented 3 months ago

Sure, if someone wants to add this, I'm all for it. I know nothing about conan, though - as you've noted, that integration was provided by someone else 😅

prince-chrismc commented 3 months ago

IIRC Since this is header only, there's no value to a conan package... adding options would change the package ID but the contents would be exactly the same. This would trick conan into rebuilding and there ways to work around this... but it's always better to aligned with the official documentation which says "define these before you include" so there's no special conan logic interfering with users.

it's likely better to configure this at the project level of your cmake :)

marzer commented 3 months ago

OH, right, of-course; if the conan integration just uses it in header-only mode, then there's no reason to bother 😅

I assumed it was consuming it as a regular pre-compiled library.

xoorath commented 3 months ago

Hey,

I haven't updated Conan recipes before so if it isn't too much trouble I'll give that a shot. 👍

For what it's worth: there is totally still value in using a package manager for header-only libraries. Personally, I like having all my third party dependencies ingested one way consistently. That's the number one benefit for me, but there are more!

Even in header-only mode if the Conan recipe defines something like TOML_EXCEPTIONS=0 when an option is set: our build systems using that recipe can have that defined automatically for our targets. That's a big upside IMO.

marzer commented 3 months ago

For what it's worth: there is totally still value in using a package manager for header-only libraries.

Nobody is telling you not to use a package manager for header-only libraries. We're saying there's not really much value in adding excessive configuration to the package manager for it (much less than 'regular' libraries), since you can set the flags in your build system directly. You can probably already propagate that right now using whatever build system you have conan integrated into.

prince-chrismc commented 3 months ago

Thanks marzer for clarifying. I preach package management (and devops)! here's a list i wrote of most of the package managers in the ecosystem.

I do understand the appeal of managing everything through Conan, the options API is the most obvious place. however that has ramifications on the package graph and might cause downstream recipes to rebuild unnecessarily. It's usually a cost you dont want to pay.

I would actually recommend using the cxxflags in a profile or from the consumer's recipe depending on how many build graphs you have.

PS I used to be on the Conan team between that original recipe and today :)

xoorath commented 3 months ago

Thanks both! Appreciate the clarification.

I think I do want to expose options in Conan so for my use case I can indicate I depend on tomlplusplus but don't want exceptions all in the same place through Conan.

If that doesn't sound like something you (or your users) would want, I can totally just define that in my build system instead of through conan options.

If you think it's reasonable to do, though: would you want that to be just some subset of options, or would you want all options? Do you care?

marzer commented 3 months ago

would you want all options

Definitely not all of them - there's quite a lot! Opting out of exceptions is probably the only one that has broad appeal, methinks.