Enable users to customize cmake and ninja versions with bzlmod

[mrkkrp]

There seem to be no way to override the default cmake (or ninja) version with a tag, although it looks like that was the original intention behind the bzlmod extension. The problem is

• The root module is explicitly skipped because of the conditional if not mod.is_root:
• The use_repo and register_toolchains invocations in MODULE.bazel of rules_foreign_cc have to mention the default version of cmake because it is part of the repository name that provides the toolchain (why?) and so MODULE.bazel is incompatible in the case of an override by the user.

This PR resolves both these issues.

[google-cla]

Thanks for your pull request! It looks like this may be your first contribution to a Google open source project. Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA).

View this failed invocation of the CLA check for more information.

For the most up to date status, view the checks section at the bottom of the pull request.

[jsharpe]

Good catch on this. I think how I would like to see this fixed is in a similar way to #1105 where the toolchains are moved into a hub repo which can be generated with all the user requested toolchain versions in it which should remove the need for the use_repo call to need to bring the repos into scope in the parent module.

[jsharpe]

We need to maintain this check - only the root module should declare the versions of the tooling.
Good catch though on this being inverted; this should be if mod.is_root

[mrkkrp]

The way it is written in this PR, we allow any module to declare a version of the tooling but the module that is closest to the root has the precedence. Is there a reason to strictly limit these version declarations to the root module only?

[jsharpe]

I think we should be able to support registering more than one version of cmake in a build. We should probably generate a version specific constraint that we apply to the toolchain targets so that a user can select a version constraint that way if they desire.

[mrkkrp]

We do support declaring more than one version of cmake, but each such declaration should be in a different module. Here we simply guard against multiple declarations in the same module.

[jsharpe]

By removing the version number from the generated names though you will get duplicate definitions if a module defines one version and then the root requests a different version. This is also the rationale for restricting definitions to the root module at least for the time being.

[mrkkrp]

While we do allow specifying several versions of cmake/ninja, only one version wins—the one closest to the root module (see break in the "then" clause). Also, there can only be one version per module, so this should be very close to the old logic with the exception that the user can actually configure the versions.

[jsharpe]

Good catch on this. I think how I would like to see this fixed is in a similar way to #1105 where the toolchains are moved into a hub repo which can be generated with all the user requested toolchain versions in it which should remove the need for the use_repo call to need to bring the repos into scope in the parent module.

Actually I have to say I didn't get as far with #1105 as I'd remembered and had only moved the toolchains so that I can register them via :all rather than listing individually. My intentions were to move it into a generated repo to remove the need for the use_repo calls.

[mrkkrp]

@jsharpe Is there anything I can do to help this progress? What do you think about my replies above?

[jsharpe]

@jsharpe Is there anything I can do to help this progress? What do you think about my replies above?

Apologies the limiting factor here is my time availability to work on this as I only maintain this in my spare time.
I think really the approach we should take is to generate a hub repo that references all the other versioned repos in the same way that rules_python does so that we can generically register all the toolchains via a //:all target. I've started to work on this in #1158 but haven't found enough time to completely feed through all the refactoring required for this. If you have bandwidth available to pick up that refactoring of the generated toolchains then that'd be great..

[mrkkrp]

I think really the approach we should take is to generate a hub repo that references all the other versioned repos in the same way that rules_python does so that we can generically register all the toolchains via a //:all target.

But if there is only one version of a given tool that is selected in the end, does it make a difference whether its repo name includes the version or it is generic? In any case, this patch solved our immediate problem and we don't think that we will have much more time to contribute to a more elaborate redesign.