Re: [Proposal] Optional Toolchains

[John Cater]

Adding both +Greg Estren and +Tom Rybka as domain reviewers.

Also sending this to the Rules Authors SIG, since you will certainly be interested.

Thanks!

John Cater

On Mon, Jan 24, 2022 at 9:20 AM John Cater <jca...@google.com> wrote:

It's been a requested feature for a while, so I'm looking at adding the capability for optional toolchains in toolchain resolution, instead of the current mandatory system.

There are two possible approaches outlined, please take a look and let me know your preferences if you are interesting in writing rules that use toolchains.

Proposal link: https://github.com/bazelbuild/proposals/blob/main/designs/2022-01-21-optional-toolchains.md

Thanks!

John Cater

[Greg Estren]

Review comments:

1. Performance question: missing mandatory toolchains, if they still exist, would still be reported before dependencies are computed? i.e. here? In other words, is performance loss possible due to "missing toolchain" errors triggering only *after* the transitive subgraph is constructed?
   
   
2. For Changes to Toolchain Resolution Logic, the subtle implication is platforms matching the same # of toolchains revert to priority-order selection. While you say as much, could you explicitly call out the most generic logic:
   
   selectedPlatform = filter(registeredPlatforms, hasAllMandatoryToolchains).sort(numOptionalToolchainsMatches).secondarySort(priorityOrder).chooseFirst() 
   
   I'd also define, briefly, what "priority" means.
   
   
3. What would be the default for mandatory?
   
   
4. For the second option, why do rule writers need to check the Bazel version? Wouldn't that outright fail for old Bazels since config.toolchain_type wouldn't resolve?

[Tony Aiuto]

Option 2 is the right choice by this reasoning:

• Assume option 1, where all toolchains are optional
• Assume there is a rule for which there a mandatory toolchain and two optional ones.
• If we look at execution platforms and find 1 platform that provides the mandatory toolchain, but another that provides both of the optional toolchains, how will we know to correctly pick the one that only has 1 toolchain match?

That said.  I think we could incrementally get there with a hybrid approach.

• Quickly move to "all toolchains are optional". Rule authors should start today towards migrating rules to fail if the toolchain is missing. Getting ahead of the bazel implementation.
• For the cases where a rule only has one toolchain, the old API and change to rules is sufficient.
• We could add a flag "--incompatible_all_toolchains_are_optional" that turns on this behavior. Declare a fixed time before the day it becomes true and then removed.
  
• We continue to work through use cases of mixed mandatory/optional toolchains w.r.t. execution platform priority.

What I would like to see is that we can move forward on allowing optionality in the most common case (single toolchain) soon, without getting bogged down in API for mandatory-ness vs. execution platforms.

Question: I am presuming the proposal does not mandate that all rule declarations change. That is, under option 2, should

  toolchain_types = ["//toolchain:type1", "//toolchain:type2"],

continue to work?  I think it has too, with the assumption that each is optional (if --incompatible_all_toolchains_are_optional is set)

--
You received this message because you are subscribed to the Google Groups "bazel-dev" group.
To unsubscribe from this group and stop receiving emails from it, send an email to bazel-dev+...@googlegroups.com.
To view this discussion on the web visit https://groups.google.com/d/msgid/bazel-dev/CABdTCQVR17JOUU7pd9oV-9qqmaFmzsBWZ5z5LukpxDODhkhPMw%40mail.gmail.com.

[Tom Rybka]

LGTM