Split `:toolchain_type` to correctly cover all use cases

[Silic0nS0ldier]

PR Checklist

Please check if your PR fulfills the following requirements:

• [ ] Tests for the changes have been added (for bug fixes / features)
• [ ] Docs have been added / updated (for bug fixes / features)

PR Type

What kind of change does this PR introduce?

• [x] Bugfix
• [ ] Feature (please, look at the "Scope of the project" section in the README.md file)
• [ ] Code style update (formatting, local variables)
• [ ] Refactoring (no functional changes, no api changes)
• [ ] Build related changes
• [ ] CI related changes
• [ ] Documentation content changes
• [ ] Other... Please describe:

What is the current behavior?

Issue Number: #3795

See issue for details.

What is the new behavior?

• Split @rules_nodejs//nodejs:toolchain_type into @rules_nodejs//nodejs:runtime_toolchain_type and @rules_nodejs//nodejs:exec_runtime_toolchain_type.
• @rules_nodejs//nodejs:runtime_toolchain_type is suited towards usage in the target environment (e.g. runtime for *_binary rule outputs).
• @rules_nodejs//nodejs:exec_runtime_toolchain_type is suited towards usage within rule actions (e.g. running NodeJS directly).
• Split behaviour is disabled by default, controlled by a new attribute incompatible_split_toolchains on;
  • node.toolchain module extension tag.
  • nodejs_register_toolchains macro.
  • nodejs_toolchains_repo repository rule (private API).
• :resolved_toolchain target moved into @rules_nodejs//nodejs. This makes it possible to access a NodeJS toolchain implementation without needing to know the name of a user controlled generated repository (e.g. for use inside a genrule defined in another ruleset).
• Add new toolchain type @rules_nodejs//nodejs:compilation_toolchain_type suited to usage of NodeJS as a compiler (e.g. for snapshot generation) when the exec and target platforms need to be aligned. The exact platforms can still be different, but they should at least be compatible across OS and CPU.

Does this PR introduce a breaking change?

• [ ] Yes
• [x] No (but does add deprecations)

Other information

Extra TODO

• [ ] Update issue references to point to this PR.

[fmeum]

Instead of an incompatible attribute on repo rules and module extensions, it may be possible to temporarily make the new toolchain types label_flags that by default point to the old toolchain type for backwards compatibility. Enabling the new behavior would then require setting the label_flag to a new distinct type. That's a much smaller API surface and dropping the flag would only require .bazelrc changes, no source changes.

[mortenmj]

@Silic0nS0ldier any chance this might get the attention it needs to land? For context, we've been running with this for a few months now, and it solved a lot of the issues we had cross-compiling from Mac.

[guw]

Enabling the new behavior would then require setting the label_flag to a new distinct type.

Can someone help me understand why this needs a flag? Can't the "old" toolchain_type just become the compile/host toolchain and new ones are introduced for runtime? Meanwhile, rules_js and others can start updating their rules to use the runtime toolchain

Can the 3rd type be postponed to a second PR? I don't see an immediate need there. If yes I can work on a simpler version which keeps the existing toolchain_type and introduces a runtime type for the ones with target constraints. I would then also try to submit follow up fixes for rules_js. Thoughts?

[alexeagle]

I think this was finally resolved in https://github.com/bazel-contrib/rules_nodejs/pull/3859

[Silic0nS0ldier]

With the exception of @rules_nodejs//nodejs:compilation_toolchain_type (for the uncommon scenario where OS+CPU need to match across exec and target, like for snapshot generation) yes, although I think you were after #3795.

Thanks @guw for getting the important bits over the line!