-
Notifications
You must be signed in to change notification settings - Fork 986
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
New "conf" configuration mechanism #8266
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Like it. Go ahead.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I really like the syntax of the config in profiles. There are some missing tests like inheritance of profiles or implementing a merge with the general conan conf, but for the first iteration I think it a very good step forward! 👍
Some points:
|
I've just checked, the following worked for python 2.7:
|
conans/client/cache/cache.py
Outdated
@@ -155,6 +157,22 @@ def config(self): | |||
self._config = ConanClientConfigParser(self.conan_conf_path) | |||
return self._config | |||
|
|||
@property | |||
def new_config_path(self): | |||
return os.path.join(self.cache_folder, "conan_conf.txt") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'd prefer this to be decoupled from the cache folder.
otherwise, we introduce a vicious circle here: config has a path to the cache folder, but the config path is relative to the cache folder. it's super confusing.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is not new, the current config (and absolutely everything from the cache) is already loaded this way, because assuming it is in the cache folder, you need the cache folder to locate it. It cannot be decoupled, but maybe I don't understand the suggestion.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'd say it's not always a valid argument "it was always like that" when designing a brand new thing, especially if the approach is already known to be problematic.
first, it's logically inconsistent, as the configuration file doesn't belong to cache idiomatically (we are not caching it, we caching binaries and sources).
IMO it belongs to the config directory (which might be the same or might not be the same as cache).
second, if we want conan to configure to use a different cache folder, we can't, as config file is within cache folder, so cache folder needs to be known before loading the config, which obviously, introduces a vicious circle.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It is not an argument of "it was always like that". It is maybe a slightly incorrect naming. Lets rename the cache_folder
to home_folder
which is what it actually is. Conan codebase is referring to the Conan cache and the Conan home folder equivalently, because 97% of the home folder is devoted to the package cache. Basically, the Conan home folder contains the settings.yml, the remotes.json, the conan.db, the profiles plus the recipes and packages cache.
If we agree with that, I am very fine with renaming the Conan codebase from "cache" => "home" when it applies. But the logic implemented in this new_config_path()
is correct and in the right place.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We need something like this.
This works great for built-in tools and classes (strings resemble the import).
Are there any other approaches considered? pros and cons? What is the answer to #8266 (comment) (classes that are imported from other python files or python_requires
)? only via "user.*"
? In the global config are there different sections for host and build contexts?
About this one:
I think it is confusing given the current syntax, if the syntax were |
py2/py3 and standard parsing... if it is possible to implement it using standard parsers only for Python3 I wonder if we should implement this experimental feature only using Python3 and forget about people using Python2. IMO we can receive enough feedback from people using Python3 and in the future (Conan 2.0) we don't need to change the implementation again. |
RE: @uilianries
Good point, I'm not sure which one I favor, since I think we're now supporting upper-case things in CLI like class name for generators at the CLI.
Agree with either.
Definitely not
Good question, going to think about this one today.
I think this is also a really good point. I believe it relates to the fundamental challenge which has been discussed in the past. That is, built-in generators and toolchains versus custom ones. It's difficult to understand the slightly different behaviors between the two, both for users, and for us developing features. In particular, the built-in ones are less extensible for users. Typically, yes, if users want something slightly different from a built-in generator, they actually have to re-implement the generator themselves, just to change that one thing, and even then, they don't get all the same capability (like, custom generators from build-requires can't be passed at the command-line, you have to use the new, experimental, generator as a config strategy which then lacks versioning). I don't think we have a silver bullet solution for this problem yet, only a few ideas and each of them have drawbacks. |
Also, from discussion, I personally do not expect this to affect what the CMake build helper generates when using the Visual Studio Generator at all: After hearing @memsharded reasoning, I can now see there's a logical argument for it, but I still find it unintuitive. Instead, I would expect this to be a separate config, and I can definitely imagine cases where I would want different default values for each specified in my global configurations: |
Will it be possible by a (build) requirement to override/set a configuration value? |
Hi @madebr
At the moment it is not intended. The configuration so far is for changing things from the user side (profiles, command line, conan.conf):
That is, more or less to change things from some default values. If it is something that a This is more or less the current way of thinking, but we probably need to learn more, other use cases, keep working on the toolchains and specially in the environment-variables management in Conan, to really know if this will still hold, need to adapt, or even another mechanism is necessary. |
Thanks for the reply @memsharded
|
Reviewed:
|
For those things, changing default gnu triplet, changing MesonToolchain default variables, seems a good fit when applied in a profile. This seems exactly the kind of configuration that we want toolchain, build helpers, etc. to expose to users, so users can always move forward and escape limitations of those helpers by explicitly providing information, like the tripled. Having them propagated from an upstream requirement is a very different story. In the same way that such build_requires cannot define the settings nor the options of the consumer, at the moment it is not evident if the configuration should propagate in this direction too. I would need to understand better what are the advantages of putting that information in the |
I was thinking about the meson cross files (https://mesonbuild.com/Cross-compilation.html + https://mesonbuild.com/Machine-files.html). Using configuration info in build_requirements would also allow defining compiler tool chains in I'm putting these things here, because it looks like My request for always enabling overrides stems from meson machine entries. My apologies if this is getting off topic. |
Your example is relevant:
This are explicitly env-vars in Meson, aren't they? If they are env-vars in Meson, then I don't see making configuration for them, why would be better to have configuration instead of directly the env-vars? The idea is to be able to pass information that it is not structured/defined by the build systems, compilers or tools in general by env-vars, and avoid creating many new invented env-vars for things that are finally translated to a command line switch, or to certain logic inside a Conan tool or helper, because env-vars are dirtier for this purpose, less structured and very global (can be written in other recipes). Again, it is just a preliminary idea, we need to keep discovering use cases, learning and evolving this.
It is fine, thanks very much for the feedback, the intent of this PR is to start rolling the feature to gather this feedback and use cases. |
Responding to
Yes, if users provide their own helpers via
Not at the moment. The idea is that the global |
Changelog: Feature: Introducing a new
[conf]
section in profiles that allows a more systematic configuration management for recipes and helpers (build helpers, toolchains). Introducing a newconan_conf.txt
cache configuration file that contains configuration definition with the same syntax as in profiles.Docs: conan-io/docs#1993
Profiles can have:
Profiles confs:
A new file conan_conf.txt (name to be defined, can contain):
tools.*
anduser.*
configuration from conan_conf.txt is composed with the profiles ones. The rest ascore:required_conan_version
are internal to Conan and will not travel with profiles, recipes cannot see them.Recipes and helpers access them with:
The
tools.microsoft.MSBuild:verbosity
is actually applied in 2 different helpers theCMake
one that had amsbuild-verbosity
argument and theMSBuild()
one.Things that are configuration at the moment are removed from helpers. It is not necessary that users define something like the verbosity of MSBuild in each recipe, better remove them now, and we will focus one by one if some needs to be added in the helpers explicitly (to override/force the config, or to define a different default).
At the moment, no adding command line
--config
version, but it can be added later relatively easily, I had added it before, then dropped for simplicity at this moment.