conan-io / conan

Conan - The open-source C and C++ package manager
https://conan.io
MIT License
8.25k stars 980 forks source link

[feature] Enable support for os setting in configuration packages #17215

Open NormanT7 opened 1 week ago

NormanT7 commented 1 week ago

What is your suggestion?

I really like the idea of managing conan config files across our dev team and projects using the configuration package type (i.e., conan config install-pkg) and have been trying it out the past few days.

Our development projects span both Windows and Linux. We have a bunch of profiles and settings, many of which are the same on both operating systems but also some that are unique to each system. I decided to just use a single package/recipe and use settings = ("os",) so that I could have the package method only include the profiles and settings that are relevant to the given OS. Having worked with Conan for a few years now, this just seemed like a natural choice.

This approach appeared to be working great, and I was able to create the packages in my local cache. However, when I tried to consume the packages using conan config install-pkg ... I encountered the following error.

======== Computing necessary packages ========
ERROR: There are invalid packages:
conan-config/0.0.0@scieng/test: Invalid: 'settings.os' value not defined

Initially, I though this might just be because I had set CONAN_HOME to get a blank slate without any existing configuration or something to do with our remote repo. However, even when I try it from the local cache with a functional set of Conan configuration files, I get the same issue. I'm assuming, at this point, that this was just not something that was considered in the implementation.

The configuration package type is obviously special and enabling the full suite of settings and options probably doesn't make sense. However, it would be really nice if it at least supported the os setting (or some equivalent) so that we could have a single package, recipe, and set of source that gets tailored slightly based on the os setting.

As a workaround, I switched to specifying the package name on the command line so that I can pass one name for windows and one name for linux and tailor the profiles and things that are included based off the package name. This gets to mostly the same result. However, as I go to update our CI pipelines and things, I'm finding it awkward and I'm having to update a lot of things. Whereas, if it were one package with an os setting, it would fit really naturally into our existing workflows.

Have you read the CONTRIBUTING guide?

memsharded commented 1 week ago

Hi @NormanT7

Thanks for your feedback.

This is an interesting use case. We did consider it when developing the feature, but we were a bit scared it could go a bit too much "down the rabbit hole", and decided to wait until getting some users feedback about it.

The possible approach for this case is exactly what you did, have a myconf_win and myconf_linux different configuration packages, and then let users to chose the one they want. This has the potential advantage of managing configuration for both systems at different pace. Recall that the configuration version might be even become part of the package_id of the packages using that configuration. So we thought that would be probably an interesting behavior, making this approach more attractive.

But I understand you see the other possibility more attractive. Could you please elaborate a bit more, beside that it seems more natural to the Conan practices, what advantages and disadvantages you can see? Do you foresee any potential downside of supporting this conditional-settings conan config isntall-pkg?

NormanT7 commented 1 week ago

I can certainly imagine scenarios where managing multiple configuration packages with independent versioning would make more sense, especially if they are mostly independent.

However, we are a small team building the same set of projects on both windows and linux, so we are inclined towards the idea of a single config package that is tailored slightly based on OS. If it were a normal package type, it would obviously be weird to have to use a separate package name for each OS and then have everything downstream have to deal with that fact. Admittedly, this perceived inconsistency in Conan functionality was the main motivation behind submitting this request. It just seemed like something that should be possible based on my experience with normal Conan packages.

As it is, we can use the same source and recipe and just pass the package name in on the command line. The main impact of this is everything downstream has to manage an os-specific call to conan config install-pkg. Additionally, due to the single conanfile but two package names approach, I have to use a tailored release pipeline for the config package instead of using the template used on all of the other packages. Neither of these are large impacts, just a bit inconvenient.

We haven't adopted lock files yet, so I'm not well versed with the functionality surrounding those. That being said, it seems that having a different config package name for each os would mean that a consumer would have to have a lock file for each os. If so, this would preclude the use of the conan.lock name to have it be loaded automatically.

I can't come up with much in terms of downsides of adding support for the os setting to config packages, but that might just be my lack of broad perspective on Conan and how it's used. Adding support for just the os setting wouldn't eliminate inconsistency or potential confusion between the different package types. I don't think it makes sense to adding any of the other settings, but maybe someone will come along wanting that.

memsharded commented 1 week ago

Thanks very much for your feedback.

Here is a quick proof of concept: https://github.com/conan-io/conan/pull/17217

Can't guarantee it will be moved forward, we will think about it with the team a bit more, to try analize potential downsides or risks. But as you can see the code is not complex, it was already almost prepared to implement the feature, as commented above, we had already considered this use case while implementing the config install-pkg feature 🙂

But if you want to give it a try, running from my source branch, feedback is always welcome!