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

How to set default_option based on some setting? #3519

Closed
DoDoENT opened this issue Sep 10, 2018 · 8 comments
Closed

How to set default_option based on some setting? #3519

DoDoENT opened this issue Sep 10, 2018 · 8 comments

Comments

@DoDoENT
Copy link
Contributor

DoDoENT commented Sep 10, 2018

Consider this conanfile:

from conans import ConanFile

class MyConan(ConanFile):
    name = "Test"
    version = "0.1.0"
    description = "Insert project description here"
    settings = 'arch', 'os', 'build_type', 'compiler'
    options = {
        'some_option': [True, False]
    }

    def config_options(self):
        if self.settings.os == 'Android':
            self.options.some_option = True
        else:
            self.options.some_option = False

    def configure(self):
        print( f'Some option: {self.options.some_option}')

I want the some_option to be set to True by default if OS is Android and to False otherwise. However, if someone specifies the some_option via command line, I want to use that. How to do that? I was under impression that config_options method is used for that, but when it appears it is not:

$ conan install .
Some option: False
$ conan install . -o some_option=True
Some option: False

If I remove the config_options method, then the second example works correctly, but the first one fails with ERROR: Conanfile: 'some_option' value not defined.

How to set a default value for the option, based on some setting, like OS?

I am using Conan v1.7.3 on MacOS.

@memsharded
Copy link
Member

I am afraid it is not possible to define conditional default options. The behavior you are reporting is the intended one, once an option is specified like self.options.some_option = True, it is a "hardcoded" option, and it is not overridable.
I have done a quick check, and this would be a breaking change, so it is not something that can be changed now. I suggest using the standard default_options field instead of hardcoding, and relying on profiles, making for example the Android profile set the corresponding option for the package different than the real profile. Profiles are overridable by command line, so that would get your desired behavior.

@DoDoENT
Copy link
Contributor Author

DoDoENT commented Sep 11, 2018

From the documentation:

config_options() is used to configure or constraint the available options in a package, before they are given a value.

Due to this sentence, I was under impression that config_options is invoked before the options are given a value, i.e. before command line parameters -o option=value is evaluated, thus giving the possibility to set the default value, a constraint or to delete the option before the value from command line is set. Basically, due to the above sentence from the docs, I thought the order is the following:

  • config_options() is executed
  • command-line -o option=value arguments are evaluated, whilst performing checks against restrictions given in config_options and possibly overwriting values specified with default_options attribute and values set in config_options() method
  • configure() is executed

Obviously, I got it wrong.

Can you please explain (or update the documentation) what is and what isn't allowed in config_options method? Because, from my point of view, deleting an option in config_options (as given in example in the documentation) is similar to assigning a conditional default value in the same method.

Also, can you consider adding support for conditional default values in the future (either with the described mechanism or by introducing the new method default_options() that will provide a way to assign default values to options, based on settings, or, possibly, on other options that already have their value assigned by the default_options attribute)?

I suggest using the standard default_options field instead of hardcoding ...

This does not work either:

from conans import ConanFile

class MyConan(ConanFile):
    name = "Test"
    version = "0.1.0"
    description = "Insert project description here"
    settings = 'arch', 'os', 'build_type', 'compiler'
    options = {
        'some_option': [True, False]
    }
    default_options = f'some_option={self.calculate_some_option()}'

    def calculate_some_option(self):
        if self.settings.os == 'Android':
            return True
        else:
            return False

I get NameError: name 'self' is not defined.

... and relying on profiles, making for example the Android profile set the corresponding option for the package different than the real profile.

I am already using profiles for specifying other settings and options, that are orthogonal to specific option in my specific package. Having separate profile for each combination of options for each package would cause a combinatorial explosion and is not feasable for my setup.

@DoDoENT
Copy link
Contributor Author

DoDoENT commented Sep 11, 2018

Ha, it appears this hack works correctly, at least for my use case:

from conans import ConanFile

class MyConan(ConanFile):
    name = "Test"
    version = "0.1.0"
    description = "Insert project description here"
    settings = 'arch', 'os', 'build_type', 'compiler'
    options = {
        'some_option': [True, False]
    }

    def config_options(self):
        if self.options.some_option == None:
            if self.settings.os == 'Android':
                self.options.some_option = True
            else:
                self.options.some_option = False

    def configure(self):
        print( f'[configure] Some option: {self.options.some_option}')
$ conan install .
[configure] Some option: False
$ conan install . -o some_option=True
[configure] Some option: True
$ conan install . -s os=Android -s os.api_level=16
[configure] Some option: True
$ conan install . -s os=Android -s os.api_level=16 -o some_option=False
[configure] Some option: False

The only mystery is why it is mandatory to compare with ==, i.e. why

        if self.options.some_option == None:

works, while

        if self.options.some_option is None:

does not?

EDIT: mystery solved

@DoDoENT
Copy link
Contributor Author

DoDoENT commented Sep 11, 2018

@memsharded, if that hack looks OK to you, can you consider adding that to the documentation about conditional settings and options and making it official way to handle conditional default options?

@memsharded
Copy link
Member

memsharded commented Sep 11, 2018

Yes, I agree it would be good to add it to the docs: conan-io/docs#816

Because, from my point of view, deleting an option in config_options (as given in example in the documentation) is similar to assigning a conditional default value in the same method.

The problem is that the semantics of assigning an option in the config_options() and configure() options already got the semantic of defining the final value, not the default value. It was requested by users, that wanted to enforce that, and error otherwise, because they really wanted to force those options upstream. So both things can be done, removing options or assigning values. Just that assigning values is assigning the final one, not the default (initial) one. And this is what can't be changed without breaking.

Also, can you consider adding support for conditional default values in the future (either with the described mechanism or by introducing the new method default_options()

Yes, we are thinking about possible improvements regarding the configuration of options, we are studying what could be done in a future conan 2.0 (breaking) release. This, or something close will probably be provided. I am linking this issue #3524 there too, so I think this one can be closed by now. Thanks for all the feeback!

@DoDoENT
Copy link
Contributor Author

DoDoENT commented Sep 11, 2018

Just that assigning values is assigning the final one, not the default (initial) one.

Exactly that confused me - I got the impression that the final one is assigned in configure, not in config_options.

This makes it difficult to understand the differences between configure and config_options. From the example in the documentation, it looks like this:

def config_options(self):
    if self.settings.os == "Windows":
        del self.options.shared

is executed before conan install -o Pkg:shared=True (i.e. before evaluation of -o Pkg:shared=True), thus enabling raising of an exception in Windows saying that shared is not an option for such package.

However, the fact is that it is executed after evaluation of command line option, but before the execution of configure, and also the fact that within configure you have the chance to once more assign the different, now final, value to the same option, and also to the dependencies options, makes it very unclear what is the actual difference between config_options and configure.

From my tests with the current issue, I've found that not having default option at all and not specifying it on command line, fails with error before configure is executed, but after config_options is executed, so there is obviously some difference between the methods, but it is not clear to me which one (as I said, I thought the difference was as explained in my earlier comment, but I was wrong).

Can you please improve the documentation of config_options vs configure, possibly with some tutorial-like story which better describes when to use one and when another and what is possible/allowed in one and what in another method? I think that would avoid future confusions like mine and save lots of developers' time in the future.

DoDoENT added a commit to microblink/conan-build-helper that referenced this issue Sep 11, 2018
DoDoENT added a commit to microblink/eigen-git-mirror that referenced this issue Jan 31, 2019
lmglmg pushed a commit to microblink/eigen-git-mirror that referenced this issue Jul 17, 2019
@Minimonium
Copy link
Contributor

Hey, @memsharded . Could we include this issue of conditional default_options in the Conan 2.0 project too since you mentioned previously that it would be a breaking change to fix in the current version?

@Adnn
Copy link

Adnn commented Apr 14, 2020

@memsharded conditional default values for options are also a feature we are looking for.

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

No branches or pull requests

4 participants