ninja: allow any MSVC runtime when consuming ninja

[ericLemanissier]

Specify library name and version: ninja/1.10.0

closes: #1831

CC @robert-shade

• I've read the guidelines for contributing.
• I've followed the PEP8 style guides for Python code in the recipes.
• I've used the latest Conan client version.
• I've tried at least one configuration locally with the
  conan-center hook activated.

[conan-center-bot]

All green in build 1 (939f90f9e93147075de2e63927841cd4d064c595)! 😊

• ninja/1.10.0: Generated 3 packages. All logs here

[madebr]

This is touching the same issue as cmake: #1783 (review)

Why disable a perfectly working configuration in a recipe?

[robert-shade]

This was indeed influenced by #1783. The original issue that one was trying to solve was removing the dependency on the debug runtime for tools. It sounds like this is a deeper issue with how conan/CCI handles tools, so we may just want to cut the if statement out altogether.

[uilianries]

I didn't understand how this could solve the problem. The settings is solved before running build(), it should result in the same result.

[ericLemanissier]

It does not change anything for building ninja, but it allows consumers to use an existing ninja binary regardless of compiler.runtime

[patricia-gallardo]

Context: I use conan-qt - my build options mean that I need ninja during the build of conan-qt - that worked until the change was made, now I can't build:
ERROR: ninja/1.10.0: Invalid configuration: Only MT MSVC runtime is supported

[jgsogo]

This is totally aligned with this issue (conan-io/conan#6952), we are aware of the problem, but we don't know how to fix it yet. We would like to know that the recipe won't build in advance... but meanwhile, raising in the build() method is the best we can do.

[madebr]

This pr does not fix #1831 because the ConanInvalidConfiguration will still be raised, just somewhere else.
#1831 is asking to remove the raise altogether.

Actually, this pr changes nothing?

[jgsogo]

This PR is moving the ConanInvalidConfiguration from the configure() method to the build() one. This way, the exception is not raised when evaluating the graph, but only if this package is going to be built. This is important, otherwise, any graph with this package inside will fail for any profile with compiler.runtime=MD.

Here in CCI this trick works because we are iterating first the profiles with MT, so profiles with MD are discarded (they compute the same package-id). Otherwise, we would get an error in the conan create command and the job would fail because we are not handling there the exception ConanInvalidConfiguration

[ericLemanissier]

It fixes the vast majority of users which will just reuse ninja binary downloaded from conan-center (which includes 99.99% of CI setups). But I acknowledge that it does not fix MSVC users which force rebuilding ninja from sources. This PR requires these users to set compiler.runtime=MT when forcing ninja build from source, but grants the use of ninja to everyone without having to redistribute msvc runtime.
The alternative (removing the raise altogether) makes it easier for people forcing ninja build from source, because they can use any runtime, but then consumers depend on MSVC runtime being installed.
IMHO the trad-off of this PR is fair (and more explicit)

[madebr]

This PR is moving the ConanInvalidConfiguration from the configure() method to the build() one. This way, the exception is not raised when evaluating the graph, but only if this package is going to be built. This is important, otherwise, any graph with this package inside will fail for any profile with compiler.runtime=MD.

@jgsogo
Thanks for the explanation. That makes sense.
It's like options of dependencies that are not initialized in configure. (Something to fix in conan 2.0?)

@ericLemanissier
I understand the trade off.

Maybe a recipe should be able to set a default configuration, when used as a build requirement.
Then, if conan cannot find a package according its build profile, it can try the default profile.

[uilianries]

Thanks for the explanation, now it makes sense

[madebr]

@jgsogo @uilianries @danimtb
Is there a issue about the problem about how to avoid build matrix explosion for installer-only packages such as ninja and cmake?

[jgsogo]

There is an internal issue for the CCI infrastructure. The way to go is quite straightforward, but it is not so easy to implement:

• iterate the profiles in order: first Release/MT
• these build_requires recipes will compute the same package-ID for many profiles as they are removing the build_type and the compiler.
• CCI will build the first profile found for every different package-ID

That way there is no need to block some configurations and the end user can use any profile to build the package from sources.

---

@madebr

@jgsogo
Thanks for the explanation. That makes sense.
It's like options of dependencies that are not initialized in configure. (Something to fix in conan 2.0?)

Totally, we will try to provide a better graph implementation for Conan 2.0.

@ericLemanissier
I understand the trade off.

Maybe a recipe should be able to set a default configuration, when used as a build requirement.
Then, if conan cannot find a package according its build profile, it can try the default profile.

This is more like the compatible package feature, isn't it?

[madebr]

@jgsogo

I'm thinking about a conan feature that allows a recipe to tag itself,
when used as a build requirement, to use a certain configuration.

Something like the following:

class SomeProgramConan(ConanFile):
    def as_build_requirement(self):
        if self.settings.compiler == "Visual Studio":
            self.override.settings.compiler.runtime = "MT"
        self.override.settings.compiler.build_type = "Release"

So let a recipe modify its own settings object.
With some extra conan features, this can be extended to allow a build requirement to be built with another toolchain. (e.g. build with mingw, even if the host compiler is Visual Studio)

e.g.

class SomeProgramConan(ConanFile):
    def as_build_requirement(self):
        if self.settings.compiler == "Visual Studio":
            self.output.info("some_program must be built with gcc")
            self.override.settings.compiler = "gcc"

If conan would allow to have multiple compatible build profiles, then conan can iterate those and choose one to build this build requirement.

[jgsogo]

Hi, IMO this is adding too much magic, recipe will be taking too many decisions without the user knowing about it. Yes, it is a problem if a recipe doesn't build with certain configuration because it will invalidate all the graph. If it is a build_requires the problem has an easy workaround: build the package first, use compatible_packages or remove settings, so the already available binary will be used (no need to compile with that invalid profile).

Another alternative would be for the user to override settings from specific packages. It is explicit, and IMO preferred from the user perspective:

-s someprogramconan:compiler=gcc

---

There are other plans in my head (but they are not in the roadmap.... yet 😄 ). Not a solution, but it might help with some graphs:

class Package(ConanFile):
    def build_requirements(self):
        self.build_requires("compiler/...", context="compilers")
        self.build_requires("tool/...", context="helper")
        ...

$ conan create <package> --profile:host=host --profile:compilers=xxxx --profile:helper=yyyy

basically, you can use whatever identifier you want for a context and it will expand to a new graph. Maybe context="build" has a special meaning and the environment is propagated from that one.

[madebr]

That looks nice.

The future's bright! 😄

[ericLemanissier]

unfortunately this PR had zero effect because of #2049: there is no windows binary available. Why ?