Skip to content
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

Add test to show that changing option values in config_options is a no-go #16672

Merged

Conversation

AbrilRBS
Copy link
Member

@AbrilRBS AbrilRBS commented Jul 15, 2024

Changelog: Omit
Docs: Omit

Probably won't merge it, just wanted to be able to show it and have a place to link from CCI

cc @uilianries - as part of my review for https://github.com/conan-io/conan-center-index/pull/24627/files#diff-a4348270915a235071ac4a599f4c2cc7d2427bb96444198c014796e55edff14eR54

@uilianries
Copy link
Member

Is it expected, right? It's same behavior since Conan 1.x.

@jcar87
Copy link
Contributor

jcar87 commented Jul 15, 2024

This is how config_options has always worked - if an option is given a value (conditional on something), it will take that value "as default" as if it had been defined in the default_options dictionary. Much like the default options dictionary, if the user provides a value in the CLI, then that's the value that taken - it makes sense: the user gave us a value, so we use it.

It can be useful to have a default option value conditional on a setting - it can't be conditional on other options, because the options dont have a value (yet).

What is less clear is hat the behaviour is when an option has a default_option and is also conditionally set in config_options - not sure that bit was ever documented.

But changing this behaviour is likely to break things.

@memsharded
Copy link
Member

This was different in Conan 1.

The self.options.foo = 2 had higher priority than the profile value in Conan 1

@AbrilRBS AbrilRBS marked this pull request as ready for review September 24, 2024 14:18
@AbrilRBS AbrilRBS added this to the 2.8.0 milestone Sep 24, 2024
@memsharded memsharded merged commit 136561b into conan-io:develop2 Sep 24, 2024
2 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants