msvc_runtime_flag should not break when expecting a string value

[uilianries]

Changelog: Fix: msvc_runtime_flag returns empty string instead of None.
Docs: conan-io/docs#2363

The helper msvc_runtime_flag works fine when running on Windows, but it returns None when something is not expected, so the follow condition is fragile:

    if "MT" in msvc_runtime_flag(self):
        pass

On Windows it can work, but for any other OS/compiler it will result:

if "MT" in msvc_runtime_flag(self):
TypeError: argument of type 'NoneType' is not iterable

The possible solution is an extra validation, but it makes the code more polluted and can result in prone error sometimes:

    if self.settings.compiler in ["Visual Studio", "msvc"] and "MT" in msvc_runtime_flag(self):
        pass

If we return an empty string, it be avoided on Linux/Mac/other compiler.

• Refer to the issue that supports this Pull Request.
• If the issue has missing info, explain the purpose/use case/pain/need that covers this Pull Request.
• I've read the Contributing guide.
• I've followed the PEP8 style guides for Python code.
• I've opened another PR in the Conan docs repo to the develop branch, documenting this one.

^{Note: By default this PR will skip the slower tests and will use a limited set of python versions. Check here how to increase the testing level by writing some tags in the current PR body text.}

[memsharded]

The check should be in any case is_msvc() and "MT" in msvc_runtime_flag()

My concern here is why a recipe needs to check the flag at that low level. This utility was intended to return the flags for usage, not for checking the runtime, for that you typically check the settings instead. Can you give some example of this usage? I am fine accepting this, no problem, I mostly wonder if we are missing something and I would like recipes to be as clear as possible and seeing "MT" in xxxxx seems pretty low level, so maybe we can abstract it in a better way.

[uilianries]

Conan Center Index is good example, when some project doesn't support MT and you need to raise ConanInvalidConfiguration.

Some real cases:

https://github.com/conan-io/conan-center-index/blob/28846bc318db1c670689c430eaaad1c2d90a72f2/recipes/fast-cdr/all/conanfile.py#L51
https://github.com/conan-io/conan-center-index/blob/3e0878b5aedbfdf28046ab039b9f3a66db3e8f4f/recipes/opencv/2.x/conanfile.py#L96
https://github.com/conan-io/conan-center-index/blob/06762d9a0396a0cf3e0931a2ca76c70a3ff1ba0a/recipes/exiv2/all/conanfile.py#L107
https://github.com/conan-io/conan-center-index/blob/78169141552f841066ca4a4d19b431ff8ae90ed0/recipes/libsodium/all/conanfile.py#L66

[memsharded]

Conan Center Index is good example, when some project doesn't support MT and you need to raise ConanInvalidConfiguration.

Yes, this is exactly what I mean, that those checks should be higher level, something like:

if msvc_static_runtime(self): 
     raise ...

Furthermore, those checks seems to be not specific to those recipes, but a general thing. Recipes shouldn't protect against every possible misconfiguration, the validate() should be applied for things very specific for that recipe, like "this library really, really won't work on Windows" or "this library really needs c++17 to be compiled and used".

So:

• If those checks are generic, to all recipes, lets make them be part of Conan (could be a hook that checks profiles correctness before being used)
• If the checks are very specific to those recipes, lets do a higher level is_msvc_static_runtime()?

[uilianries]

Recipes shouldn't protect against every possible misconfiguration, the validate() should be applied for things very specific for that recipe

True, but if we don't apply those filters, they will fail on CI.

If the checks are very specific to those recipes, lets do a higher level is_msvc_static_runtime()?

It's a possibility indeed. I can create a new PR providing such helper.

The helper msvc_static_runtime is already dedicated to Visual Studio and msvc, so if you are calling it, it's because you want to validate only with these compilers. I don't think we need to add new "if" condition to check the compiler, when the method is very specific, it looks a redundant condition.
Also, if you are writing a recipe on Windows, everything will be fine, but if you forget to validate the compiler, and run on Unix, the recipe will be broken. For example: https://github.com/conan-io/conan-center-index/blob/17bcdec7a28b6ea3602c1bca270f87cdc557e56a/recipes/glfw/all/conanfile.py#L94 we would not need that compiler validation.

[memsharded]

True, but if we don't apply those filters, they will fail on CI.

That is a failure of C3i. It shouldn't be generating configurations of shared + static runtime if that is not a valid configuration. It is still not the responsibility of the recipe to handle these cases.

[uilianries]

That is a failure of C3i. It shouldn't be generating configurations of shared + static runtime if that is not a valid configuration. It is still not the responsibility of the recipe to handle these cases.

C3i can not guess it, it need to be informed by ConanInvalidConfiguration. That's the only exception which is not considered an error and won't break the CI flow. But anyway, I think the better approach will be is_msvc_static_runtime

[memsharded]

C3i can not guess it, it need to be informed by ConanInvalidConfiguration. That's the only exception which is not considered an error and won't break the CI flow.

It depends. If this is an error for all recipes, it is an invalid configuration in general, then recipes shouldn't handle it. C3i can totally exclude this profile from generation completely.

[memsharded]

This can be merged, is correct.
But lets keep an eye on the recipes and ConanCenter:

• If every recipe in ConanCenter should include a validate() check for a shared-MT combination, that is a big smell, that such profile should be excluded from generation completely, shouldn't be a recipe thing
• If a helper is_msvc_runtime_static() helps, we can certainly add it.

[uilianries]

If every recipe in ConanCenter should include a validate() check for a shared-MT combination, that is a big smell, that such profile should be excluded from generation completely, shouldn't be a recipe thing

Only few projects. As you may know, Conan does a good work finding configurations which are not tested/supported by upstreams. Usually they build a single configuration on Windows and Linux, and that's it. We explore all possibilities, so this kind of situation is expected.