Potential bug with PackageOption @value.setter

Conan version: master

What I found: I was working today on creating some OpenCV packages using the following package: https://github.com/conan-community/conan-opencv

I used CPT to create two builds: one with GTK and one without:

from conan.packager import ConanMultiPackager

if __name__ == "__main__":

    builder = ConanMultiPackager(build_policy="missing")

    # Without GTK
    builder.add( settings={'build_type': 'Debug'}, options={ "opencv:gtk": None } )
    builder.add( settings={'build_type': 'Release'}, options={ "opencv:gtk": None } )

    # With GTK
    builder.add( settings={'build_type': 'Debug'}, options={ "opencv:gtk": 3 } )
    builder.add( settings={'build_type': 'Release'}, options={ "opencv:gtk": 3 } )

    builder.run()

What I noticed, however, is that the recipe was not honoring the gtk=None setting. Looking at the recipe I saw:

cmake.definitions['WITH_GTK'] = self.options.gtk is not None

The 'gtk is not None' expression was evaluating to true, even when the option was set to the value none. Digging into the PackageOption class, I found that the underlying type of self.options.gtk._value was 'str', which I believe was being caused by the following line: https://github.com/conan-io/conan/blob/3b74f2ac2f1e8b442925d1e09b9b53738402612b/conans/model/options.py#L358

I believe this is why the 'is not None' comparision was always returning True.

I'm not sure whether or not this is simply working as expected, and the error lies with the recipe, or if this 'cast' to str should be handled on a case by case basis depending on type. A quick example:

def value(self, v):
        if v is None:
            self._check_option_value(v)
            self._value = None
        else:
            v = str(v)
            self._check_option_value(v)
            self._value = v

The problem with the options is that we have to make them consistent for all the inputs a consumer can use to provide values for them: inside the recipe and command line. And when the input is the command line it is always a string, so for consistency we are converting them always to strings... and this is the origin of some of the problems/misunderstandings/flaws/...

On the other hand, it's not the same to make the comparison using is or == in Python, the first one (is) is going to retrieve always False because self.options.opt is a typed object while the expression on the right is not going to be a PackageOption object, it will usually be a value ("None", "shared",...). Using == will call the overloaded operator where we are converting the expression on the right to string before making the comparison.

TL;DR: I think that the self.options.gtk is not None should be self.options.gtk != "None" or just bool(self.options.gtk) (the conversion to bool is overloaded too).

Just for reference, some time ago I did a matrix trying to evaluate every combination of options and values, you can find it here https://github.com/conan-io/conan/issues/3620.

conan-io / conan

Potential bug with PackageOption @value.setter #5255