conan support

[tzcnt]

Let's try to figure out how to get the TMC_IMPL requirement out of the conan release.

[RazielXYZ]

So, continuing the discussion, here's an old discussion about stb on conan conan-io/conan-center-index#8398

There wasn't really any consensus, and stb still requires the implementation defines from consumers, in its current version in conan.

That said, stb is specifically a C "header-only" library, which is why, I suppose, it's forced to use such trickery - in modern C++ (which TMC obviously rather requires), we can put pretty much anything and everything in headers without any of that.
A cursory glance at a few of TMC's .ipp files makes me think you could basically just shove "inline" in front of everything and be done with it, making it fully and properly header-only - any reasons why that was not what you went with in the first place?

[tzcnt]

The .ipp separation is only to improve compile times (in fact I had a goal to take this even further: #67). It's not necessary for the implementation. However I have not done a benchmark of how much benefit that actually provides. So that provides a good "fallback" solution if we can't find a better way.

My only other concern is that if the user sets some config options (which become build macros) that these are applied consistently for not only TMC but also all packages that depend on TMC. And the choice of what macros to set should be specified by the end user in their config, not in some other package's config.

Do I understand correctly that the only problem with the way STB does it right now is that some library that depends on STB could also include the IMPL macro and then there would be an ODR violation? Can we find an example of a library that actually does that in the conan index?

It's getting late here and I need to sign off for the night but I'll take a deeper look into this tomorrow.

[RazielXYZ]

My only other concern is that if the user sets some config options (which become build macros) that these are applied consistently for not only TMC but also all packages that depend on TMC. And the choice of what macros to set should be specified by the end user in their config, not in some other package's config.

This is quite easy to handle (with conan, at least). If TMC would be a true header-only library (all the method implementations in .ipp would be inline) then the config options would be propagated to everyone through the self.cpp_info.defines of TMC. If some different libray uses TMC and absolutely requires a certain config to be a certain way, that would be defined in its own recipe and it would force consumers to basically use that config as well (and if they don't, conan will not resolve the dependency graph and tell them why).

If, instead, the ipp files become cpp files and TMC becomes a compiled lib, things would work... basically the same way as far as conan is concerned, except TMC itself would also be built with those configs, generating different packages for every combination. Things that need it would have to all use the same config options and conan would not allow builds with different options set to coexist in the same dependency tree.

Do I understand correctly that the only problem with the way STB does it right now is that some library that depends on STB could also include the IMPL macro and then there would be an ODR violation? Can we find an example of a library that actually does that in the conan index?

I haven't found an example of it actually happening so far, but there is at least an example of a conan package being modified to per-emptively avoid it conan-io/conan-center-index#4449 (which is the one mentioned in the discussion before as well)

[tzcnt]

That one is interesting. They basically force-provide a single implementation and then neuter the capability of any other library to provide an implementation? It seems like this provides the best of both worlds (reduce compile times, and allow multiple dependents). The downside being that it's "hacky".

On the defines topic: I found this conan-io/conan#14809 (comment) which seems to imply that defines aren't always properly viralized to all consumers?

BTW I know this is a strawman at this point as I don't expect there to be any secondary dependencies just yet. And it's possible that the linked Conan issue will be resolved before this becomes a problem. I just want to make sure I understand what the implications are.

Since you seem knowledgeable about build systems, I had some questions over here: #205 (comment) it seems like modules could also solve the "long compile times of duplicated inline functions" issue in the long run? If so then I think I'm even more on board with just making everything inline, and letting modules be the longer term solution.

[RazielXYZ]

cpp_info.defines does propagate to all consumers; not rebuilding said consumers when a required dependency's defines change due to an option though is interesting. I don't think I've ever seen that affect any of the libraries/recipes in conancenter so far, but it does seem to have the potential for ODR violations, assuming the consumers actually end up with parts of the dependency compiled in.

As for the compile time impact of inline functions, I'd say this would have to be benchmarked. I haven't really tried messing with modules at all yet, as the supposed benefits don't outweigh the pain of dealing with them so far, at least for me, so I also haven't benchmarked compile time differences between a non-module and module version of something like this. I do know that they are supposed to help, but the few benchmarks I've seen around so far didn't make it seem like a big impact.
Generally it's heavy template instantiations that really slow down the compile times. inline functions can probably slow it down somewhat due to duplication, but in my experience more so slow it down due to the inability to parallelize the compile (since all those inline functions that could have been compiled into/by different TUs in parallel instead typically all get compiled into one TU). But again, I'm not entirely sure since I haven't really seen any good benchmarks on the topic.

[jcar87]

Chiming in as maintainer of Conan Center.

Zooming out a little - our goal is to provided libraries "as-is", so we make every effort to provide them as they are documented by the library authors. So if a library is documented as a "header-only" library that requires the consumer to define a macro in the file doing the #include - then the recipe will package the headers and will not automatically propagate any macros, this will be left to the consumer at consumer site to simply use the library as it is documented.

Deviations such as conan-io/conan-center-index#4449 would not be accepted today - and they'll be rejected if we cannot find evidence of the same approach being followed in other package repositories. That is, we want to avoid a situation where a library is packaged by other repositories, but Conan Center is the only one that packages them as a libraries when no one else does. Adding a hand-rolled, ad-hoc build system that does not exist elsewhere also goes beyond that we're comfortable maintaining and would only be done exceptionally.

We've also had issues reported in the past where header-only recipes propagate macros to consumers based on some option, removing the ability for consumers to have the macro defined to different values in different translation units (for some headers/libraries, this is perfectly acceptable). May not be the case here, but we also want to avoid this.

Regarding ODR violations - I feel that sometimes there are pre-emptive efforts to avoid multiple definitions, without any evidence of specific issues actually happening.

In a Conan dependency graph, a recipe will only appear ONCE in the graph, unless explicitly hidden by consumers (and then again, they should only do so when they ensure the symbols are not publicly visible). So there can be cases in which headers included in multiple translations units with implementation are not a problem, in the sense that the implementation is the same as it come from the same exact headers, if we are dealing with the type of symbols that the linker can de-deduplicate without risk.

There are, of course, bigger risks when there are multiple instances of symbols and they are different (either because they are actually are, or because they are affected by other macros / compiler flags in each translation unit), if we're dealing with global state - etc.

So as far as we're concerned, a recipe for TooManyCooks would be a trivial recipe that packages the headers, and TMC_IMPL is not a concern for the recipe - it is for the consumers to simply follow the readme.

If there are legitimate concerns around ODR/duplicate symbols, I suspect there are linker techniques that can be used to cause the linker to error-out immediately if this situation is detected. However my unsolicited advice here would be to just turn this into a library in this repository, add a build system (with tests) - that can be packaged as such.

[tzcnt]

I've created a conan center recipe here: tzcnt/conan-center-index@971121c which covers all the configurations, and offers some guidance as to which are appropriate to be set by a dependent recipe. This guidance is actually conservative; the only config which would actually be breaking if defined by a dependent recipe would be standalone_compilation or windows_dll if they provide their own implementation. Given that the default behavior is now header-only, there's no reason for anybody to do this.

It's not ready to merge just yet as I haven't tagged a proper v1.5, but I would appreciate an early review if you have any feedback. I also have a couple questions.

About the package_id function: is self.info.settings.clear() the correct way to make the package_id depend on the options only? Do I even need a settings block since there's no build? It seems that other header-only libraries provide these settings.

What's the best way to support an optional Asio dependency? The tmc-asio integration is hosted in a separate repo https://github.com/tzcnt/tmc-asio so I can see how that could be created as a separate recipe, which depends on TMC and inherits its options. However it might be a bit cleaner if I could just offer a with_asio option on the main TMC package which also fetches this additional repo source and exposes it. I'm just not sure how to do that.

Another thing: tmc-asio can support regular Asio or boost Asio, so is there a way I can detect if the user has either one of these elsewhere in their dependency tree (and then just depend on whichever they already using)?

[RazielXYZ]

It would be great if @jcar87 can chime in, but that is quite a lot.
Is there any reason why dependent recipes should never configure those listed options (other than standalone_compilation and windows_dll) if/when TMC is using full header-only mode?

The windows_dll use is quite weird as well. Normally, built libs that support building as shared will have a "shared" option in their conan recipe, which builds and packages them as shared libs (dll or so or dylib). But this expects the end-user to use that option, then also provide an implementation file to build the dll, and then manage that dll themselves. I'm not sure if there's any other packages that do this or offer such an option. It would make far more sense if the recipe would just build and package itself as a lib if that was desired.

For the asio dependency, having it as a separate recipe is relatively fine, but it being hosted in a separate repo in the first place means that whether it's in the same recipe or not, versions would have to be synced. If you want to have it in the same recipe you can, by having extra info in the conandata.yml, as another set of url and hash that you grab in source(). This is how I was going to do it for bgfx: https://github.com/conan-io/conan-center-index/pull/24423/changes#diff-da28172cf047911ad40220cca6bd003e8060271e92b0bfce3a35ee7720fcf25e
Alternatively, if you have it as a separate recipe, the most logical thing to do would be to add tmc-asio first and then have the main tmc recipe provide an option that adds tmc-asio to its requires, but that is also awkard and makes it so any update to tmc's recipe (to add a new version, for example) would potentially need an update to tmc-asio first.

For detecting which version of asio someone is using, you could just use __has_include and default to whichever one you want if both are found, and adjust your flag accordingly at that point if needed. Here's an example of glaze doing this https://github.com/stephenberry/glaze/blob/main/include/glaze/ext/glaze_asio.hpp#L6
I don't think there's a way for a recipe to access the whole dependency tree, just info about their own dependencies, since accessing the whole thing would require a second pass theoretically (and because that could then alter the tree through conditional option or requirements changes, it'd be potentially infinite). I might be wrong though, since conan does compute the tree before it starts building the dependencies.

[jcar87]

It would be great if @jcar87 can chime in, but that is quite a lot.

Same as my earlier comment - recipes for libraries that are distributed as header files only, should be a trivial recipe that provides the headers. Options should not be used if they don't alter the contents of the recipe. And more specifically for header only, options should not be used to propagate macro definitions to consumers.

[tzcnt]

Is there any reason why dependent recipes should never configure those listed options (other than standalone_compilation and windows_dll) if/when TMC is using full header-only mode?

These aren't configurations that are useful to library developers, because they either depend on the target system, or are for debugging purposes.

Same as my earlier comment - recipes for libraries that are distributed as header files only, should be a trivial recipe that provides the headers. Options should not be used if they don't alter the contents of the recipe. And more specifically for header only, options should not be used to propagate macro definitions to consumers.
For a header-only library where macro definitions affect the ABI of the exposed inline methods, it MUST propagate the macro definitions to consumers. Otherwise the inline method definitions can be different across translation units.

I found quite a number of non-header-only libraries that use cpp_info.defines extensively to propagate macro definitions to consumers. So, what if I ship with the build mode being STANDALONE_COMPILATION (and include the build file in the recipe, it's trivial). Then this is not a header-only library but a regular static library. Then it's OK to propagate some macros? I don't really see the distinction here - the downstream impact of having a bunch of macros defined seems the same either way.

[jcar87]

For a header-only library where macro definitions affect the ABI of the exposed inline methods, it MUST propagate the macro definitions to consumers. Otherwise the inline method definitions can be different across translation units.

A header only "package", doesnt know how it is consumed, or by whom - it moves the ABI concerns elsewhere to the consumers. I could be a consumer where, at consumption point, I have multiple executables - I may different executables to define the macros differently. I can do this, because there's no shared translation units between the executables, so there's no ABI incompatible concerns. If the recipe propagates the macros, it causes two things:

• it enforces them at all points of consumption
• it creates a situation that does not exist if the header only library was consumed via other means or with other packages manager that just simply choose to make the file(s) available and nothing else.

This has already caused issues in Conan Center in the past, which is why we avoid it these days.

I found quite a number of non-header-only libraries that use cpp_info.defines extensively to propagate macro definitions to consumers.

If the upstream project for those recipes generate a cmake config file, or a pkg-config .pc file, where those macros are propagated, then the recipe will too - those are the strict usage requirements. The recipes just follow the way it is either documented publicly or the way it is packaged using the library's build system. We want to avoid constructs that dont exist anywhere else.

So, what if I ship with the build mode being STANDALONE_COMPILATION (and include the build file in the recipe, it's trivial).

It starting to sound like this library is not header only, but delivered as such to avoid maintaining a build system. We very much want to avoid tacking on ad-hoc build scripts to existing code - this is something only done exceptionally. By and large, package repositories take build scripts that are bundled with the source code of the libraries, and start from there. For true header only libraries this is usually not a problem either - they tend to have build systems that build unit tests.

[tzcnt]

A header only "package", doesnt know how it is consumed, or by whom - it moves the ABI concerns elsewhere to the consumers.

The macro propagation is necessary in the case that another library ever depends on this, and the user wants to use it as well. The user should be able to expect that an executor reference could be passed to any of the libraries' functions and work since the ABI of the executor will be the same.

If the recipe propagates the macros, it causes two things: - it enforces them at all points of consumption

Yes, this is the point. This guarantees that if one file or library refers to a function that operates on an executor, that function can be called on an executor that is exported by another library (or the global tmc::cpu_executor()).

So, what if I ship with the build mode being STANDALONE_COMPILATION (and include the build file in the recipe, it's trivial).

It starting to sound like this library is not header only, but delivered as such to avoid maintaining a build system.

The purpose of my line of questioning is to expose the fact that you are drawing arbitrary lines between header-only (not OK to export macros) and built library (OK to export macros) recipes. In truth, the need for ABI compatibility is the same either way. They are no different. So I was proposing as a workaround to your rules that I can make the library a built target.

I have a build system that builds unit tests - it's at github.com/tzcnt/tmc-examples . I deliberately exclude this from the main repo to avoid bloating it. It's not necessary as the build rules are quite simple - include the headers and define the macros identically across all consumers of the headers.

The recipes just follow the way it is either documented publicly or the way it is packaged using the library's build system. We want to avoid constructs that dont exist anywhere else.

This is the way it is documented publicly https://www.fleetcode.com/oss/tmc/docs/dev/build_flags.html . The proposed conanfile just makes it easy for users to use the library in the same way it is already documented - it just replaces the need to manually ensure the macros are defined for all consumers with the usage of options.

If the upstream project for those recipes generate a cmake config file, or a pkg-config .pc file, where those macros are propagated, then the recipe will too - those are the strict usage requirements.

Would you prefer I create a CMakeLists.txt that provides CMake options which define macros, and then propagate those macros to consumers as an INTERFACE target? Is this the "right" way to do it? (the end result will be the same though - conanfile will propagate the options to the CMake target)