Using optional C++ toolchains while maintaining compatibility with non-platform builds

[fmeum]

Description of the feature request:

It doesn't seem to be possible for a rule to simultaneously:

• declare an optional C++ toolchain dependency and gracefully handle situations in which it isn't available
• support builds with --noincompatible_enable_cc_toolchain_resolution

(ignoring potential hacks such as selecting between two rule implementations based on a config_setting backed by incompatible_enable_cc_toolchain_resolution or some other signal)

Concretely, consider the following example that tries to achieve both:

# BUILD.bazel
load(":rules.bzl", "my_rule")

my_rule(
    name = "my_rule",
)

platform(
    name = "exotic_platform",
    constraint_values = [
        "@platforms//cpu:wasm64",
        "@platforms//os:windows",
    ],
)

# rules.bzl
CPP_TOOLCHAIN_TYPE = "@bazel_tools//tools/cpp:toolchain_type"

# Inlined from //tools/cpp:toolchain_utils.bzl, modified to not fail if the toolchain isn't available.
def find_cpp_toolchain(ctx):
    # Check the incompatible flag for toolchain resolution.
    if hasattr(cc_common, "is_cc_toolchain_resolution_enabled_do_not_use") and cc_common.is_cc_toolchain_resolution_enabled_do_not_use(ctx = ctx):
        if not CPP_TOOLCHAIN_TYPE in ctx.toolchains:
            fail("In order to use find_cpp_toolchain, you must include the '%s' in the toolchains argument to your rule." % CPP_TOOLCHAIN_TYPE)
        toolchain_info = ctx.toolchains[CPP_TOOLCHAIN_TYPE]
        if toolchain_info == None:
            # No cpp toolchain found, do not report an error.
            return None
        if hasattr(toolchain_info, "cc_provider_in_toolchain") and hasattr(toolchain_info, "cc"):
            return toolchain_info.cc
        return toolchain_info

    # Fall back to the legacy implicit attribute lookup.
    if hasattr(ctx.attr, "_cc_toolchain"):
        return ctx.attr._cc_toolchain[cc_common.CcToolchainInfo]

    # We didn't find anything.
    fail("In order to use find_cpp_toolchain, you must define the '_cc_toolchain' attribute on your rule or aspect.")

# Inlined from //tools/cpp:toolchain_utils.bzl
def use_cpp_toolchain(mandatory = False):
    return [config_common.toolchain_type(CPP_TOOLCHAIN_TYPE, mandatory = mandatory)]

def _my_rule_impl(ctx):
    toolchain = find_cpp_toolchain(ctx)
    if toolchain:
        print("Optional toolchain available for", ctx.fragments.platform.platform, toolchain)
    else:
        print("Optional toolchain not available for", ctx.fragments.platform.platform)

my_rule = rule(
    implementation = _my_rule_impl,
    attrs = {"_cc_toolchain": attr.label(default = "@bazel_tools//tools/cpp:current_cc_toolchain")},
    toolchains = use_cpp_toolchain(mandatory = False),
)

With attrs = {"_cc_toolchain": attr.label(default = "@bazel_tools//tools/cpp:current_cc_toolchain")}, we get:

$ USE_BAZEL_VERSION=last_green bazel build //:my_rule --incompatible_enable_cc_toolchain_resolution --platforms=//:exotic_platform
2022/12/09 11:03:39 Using unreleased version at commit 0ca78579ffc00ddc28517ad817bf29153ccd926d
INFO: Invocation ID: 26ed0a35-10e4-4f3d-b3f0-0154b2d7345d
ERROR: /home/fhenneke/.cache/bazel/_bazel_fhenneke/ec81a4d757466008837f55d1b135a58a/external/bazel_tools/tools/cpp/BUILD:58:19: in cc_toolchain_alias rule @bazel_tools//tools/cpp:current_cc_toolchain: Unable to find a CC toolchain using toolchain resolution. Did you properly set --platforms?
ERROR: /home/fhenneke/.cache/bazel/_bazel_fhenneke/ec81a4d757466008837f55d1b135a58a/external/bazel_tools/tools/cpp/BUILD:58:19: Analysis of target '@bazel_tools//tools/cpp:current_cc_toolchain' failed
ERROR: Analysis of target '//:my_rule' failed; build aborted: 
INFO: Elapsed time: 2.684s
INFO: 0 processes.
FAILED: Build did NOT complete successfully (34 packages loaded, 147 targets configured)

Without the line, we get:

❯ USE_BAZEL_VERSION=last_green bazel build //:my_rule 
2022/12/09 11:05:04 Using unreleased version at commit 0ca78579ffc00ddc28517ad817bf29153ccd926d
INFO: Invocation ID: 8714bb4d-85b4-4469-95f7-5e324f72c759
ERROR: /home/fhenneke/tmp/bazel-optional-cc-toolchain/BUILD.bazel:3:8: in my_rule rule //:my_rule: 
Traceback (most recent call last):
	File "/home/fhenneke/tmp/bazel-optional-cc-toolchain/rules.bzl", line 29, column 35, in _my_rule_impl
		toolchain = find_cpp_toolchain(ctx)
	File "/home/fhenneke/tmp/bazel-optional-cc-toolchain/rules.bzl", line 22, column 9, in find_cpp_toolchain
		fail("In order to use find_cpp_toolchain, you must define the '_cc_toolchain' attribute on your rule or aspect.")
Error in fail: In order to use find_cpp_toolchain, you must define the '_cc_toolchain' attribute on your rule or aspect.
ERROR: /home/fhenneke/tmp/bazel-optional-cc-toolchain/BUILD.bazel:3:8: Analysis of target '//:my_rule' failed
ERROR: Analysis of target '//:my_rule' failed; build aborted: 
INFO: Elapsed time: 0.373s
INFO: 0 processes.
FAILED: Build did NOT complete successfully (1 packages loaded, 0 targets configured)

What underlying problem are you trying to solve with this feature?

Allow rules_go to have an optional C++ toolchain dependency (bazel-contrib/rules_go#3390).

Which operating system are you running Bazel on?

any

What is the output of bazel info release?

0ca7857

If bazel info release returns development version or (@non-git), tell us how you built Bazel.

No response

What's the output of git remote get-url origin; git rev-parse master; git rev-parse HEAD ?

No response

Have you found anything relevant by searching the web?

No response

Any other information, logs, or outputs that you want to share?

No response

[fmeum]

CC @katre @illicitonion

[fmeum]

I prototyped a PR that realizes the desired behavior (but isn't polished): #16968

@katre Given that 5.4.0 hasn't been released yet, do you see a way to add no-op parameters to find_cpp_toolchain and/or targets to @bazel_tools//tools/cpp that would allow rulesets to make use of optional toolchains sooner?

For example, if we added an alias of current_cc_toolchain called optional_current_cc_toolchain to Bazel 5.4.0, it could become the needed "optional" form in Bazel 6.X and rulesets could refer to it once their minimum supported Bazel version is 5.

[katre]

Notes, no particular order:

1. Please note that this is specifically for CC toolchains. Can you update the issue name and description to clarify that?
2. I see in your example that you get the error text "Unable to find a CC toolchain using toolchain resolution. Did you properly set --platforms". Are you actually using your modified version of find_cpp_toolchain?

I can see backporting a small fix to find_cpp_toolchain to make it more clear, but looking at your proposed #16968, it looks overly complicated to backport to an LTS release, and not needed at HEAD (because in HEAD optional toolchains exist).

What specific versions do you want to support, and which flags with each version? Part of the appeal of the LTS model for the Bazel team was, honestly, not doing this sort of backporting and incremental rollout of features, because it gets very complicated (and risky) to support the older versions. The optional toolchains API was added a version before the actual implementation in order to allow projects to use the new API, and then get the new features when users upgrade to a Bazel that contained them. Was this the wrong approach? What would have been a better approach, so that we can handle this next time?

[fmeum]

1. Please note that this is specifically for CC toolchains. Can you update the issue name and description to clarify that?

Good point, done.

2. I see in your example that you get the error text "Unable to find a CC toolchain using toolchain resolution. Did you properly set --platforms". Are you actually using your modified version of find_cpp_toolchain?

Yes, this text exists in two places:

1. In @bazel_tools//tools/cpp:toolchain_utils.bzl%find_cpp_toolchain, which I thus had to inline and modify slightly (simply replaced fail with return None) here in order to make it work with optional toolchains.
2. In com.google.devtools.build.lib.rules.cpp.CppHelper#getToolchainFromPlatformConstraints, which is used by cc_toolchain_alias to get the toolchain. This is the root cause of this issue: Compatibility with --noincompatible_enable_cc_toolchain_resolution means that the dependency on cc_toolchain_alias is needed, but if it is there, this method is always called and always throws this error, even if the toolchain dependency is marked optional.

The error you see in my example comes from 2., not 1.

I can see backporting a small fix to find_cpp_toolchain to make it more clear, but looking at your proposed #16968, it looks overly complicated to backport to an LTS release, and not needed at HEAD (because in HEAD optional toolchains exist).

I am not proposing to backport #16968 - the change we could backport could be as simple as adding alias(name = "optional_current_cc_toolchain", actual = "current_cc_toolchain") to @bazel_tools//tools/cpp. The backport is not meant to add any functionality or make optional toolchains available, it just makes it so that rulesets can expect this target to exist earlier (that is, already when Bazel 5 is their minimum supported version).

While optional toolchains exist at HEAD, I don't see a way for a ruleset to use them for C++ even at HEAD while remaining compatible with scenarios in which --incompatible_enable_cc_toolchain_resolution is disabled - this is why I created this issue. It is not about preserving compatibility with an earlier version of Bazel, it is just about compatibility with both values of the flag for any recent Bazel version, including HEAD. #16968 is a draft of a change that would make this possible at HEAD.

What specific versions do you want to support, and which flags with each version?

Summarizing the above, I want to allow a Bazel ruleset to declare an optional C++ toolchain dependency and simultaneously:

• make use of the toolchain actually being optional with Bazel@HEAD and --incompatible_enable_cc_toolchain_resolution=true
• do not break with Bazel@HEAD and --incompatible_enable_cc_toolchain_resolution=false

The example I gave is meant to demonstrate that this doesn't seem to be possible today, so even if we do not care about backwards compatibility, there is something that needs to be fixed.

If we do care about allowing a ruleset to do this and remain compatible with a previous version of Bazel (e.g. 5 or 6), I think that this would require a backport even if bazel-contrib/rules_go#3390 (comment) is used to only request an optional toolchain dependency if the Bazel version supports it. That is because I don't see a way to fix the example without introducing a new cc_toolchain_alias, as is done in #16968.

Part of the appeal of the LTS model for the Bazel team was, honestly, not doing this sort of backporting and incremental rollout of features, because it gets very complicated (and risky) to support the older versions. The optional toolchains API was added a version before the actual implementation in order to allow projects to use the new API, and then get the new features when users upgrade to a Bazel that contained them. Was this the wrong approach? What would have been a better approach, so that we can handle this next time?

I think that the approach was correct and adding the API a version before the feature is makes it so that rulesets can use it earlier, gracefully falling back to the old behavior but not breaking with versions of Bazel that don't have the feature. In fact, the proposal for a new cc_toolchain_alias target above is exactly of this type: It would add API to Bazel 5 and/or 6, with the actual "feature"/bug fix coming in (likely) Bazel 6.1.

The only thing that seems to have been missed is compatibility with --incompatible_enable_cc_toolchain_resolution=false. Given that this flag is toggleable even at HEAD makes it even more important for rulesets to remain compatible with both values and, as I intend to demonstrate with this issue, that doesn't seem to be possible at all with any version of Bazel.

[oquenchil]

If I understand correctly this is about the old mechanism for toolchain resolution used by C++ rules and the new mechanism not being compatible? If the old one is deprecated, we should focus on removing it instead, no need to support the flag --noincompatible_enable_cc_toolchain_resolution. Every incompatible flag is temporary and not meant for long term support.

If the problem is that without fixing the issue described here it becomes impossible to migrate, then that'd be a different story.

[fmeum]

@oquenchil The problem is that --noincompatible_enable_cc_toolchain_resolution is still the default. Even if it is flipped in Bazel 7 (not even sure that is planned), rulesets will only be able to assume it is flipped when they stop supporting Bazel 6, which will happen when Bazel 8 is released at the earliest. As a result, rulesets may not be able to use optional toolchains until the release of Bazel 8, which is probably a year away.

Adding a single alias to @bazel_tools now (that is, for Bazel 6.1), could speed this up by at least half a year.

[oquenchil]

Understood. Thanks for the clarification.

[oquenchil]

@katre For the reasons given by Fabian I think we should backport to 6.1 the simple change adding an alias to a BUILD file (not necessary for 5.4, although doing it for 5.4 wouldn't hurt either). #16968 can be checked in at HEAD and that's it, no need to backport that PR. It's only the alias that would be needed to speed up progress by a lot as explained by Fabian.

I know that this falls in the domain of C++ rules but you have a lot of expertise in this area. With those clarifications, would you be ok with backporting the alias?

[katre]

This looks very reasonable. I took a quick look at the change and it makes sense, but you should do the detailed review.