Fix toolchains and repo remapping

[dkelmer]

2398d85 broke bazelbuild/skydoc which already passes --incompatible_remap_main_repo (which gates the behavior that was changed in the PR)

To reproduce:

1. checkout bazel at 2398d85
2. build bazel
3. check out https://github.com/bazelbuild/bazel-skylib
4. devbazel test //tests:shell_tests_test_1 --incompatible_remap_main_repo

The error is:

ERROR: Analysis of target '//tests:shell_tests_test_1' failed; build aborted: no matching toolchains found for types //toolchains/unittest:toolchain_type

I assume this error is due to not passing the map in all locations that would require it, but I can't find more locations that refer to the toolchain by label

[dkelmer]

/cc @katre

[dkelmer]

Something that is probably related to the problem: the toolchain type is defined here and is then imported in all BUILD/bzl files where it's used. Repository renaming does not (and should not) apply to strings. But then when that string is passed to various attributes in toolchain processing, the string should be converted to a label and remapped then.

[katre]

Some investigation:

This fails:

$ bazel-dev test //tests:shell_tests_test_1 --incompatible_remap_main_repo

This works:

$ bazel-dev test @bazel_skylib//tests:shell_tests_test_1 --incompatible_remap_main_repo

Why? Because, when Bazel loads //tests, the toolchain type is //toolchains/unittest:toolchain_type (the repository @bazel_skylib is being remapped back to @). When Bazel loads @bazel_skylib//tests, the toolchain type is left alone as @bazel_skylib//toolchains/unittest:toolchain_type.

Trying both forms of this does work with bazelbuild/rules_rust. So next step is to figure out what rules_rust is doing differently with the toolchain type.

[katre]

I think I see the difference: rules_rust defines the toolchains in a separate (local) external repository, so the toolchain types there are also fully-qualified labels, not local labels. Trying that change now.

[katre]

Okay, looking at rules_rust was a red herring. Actual diagnosis is from debugging:

Here is the flow:

• The rule is defined as _shell_array_literal_test
  • This requires the toolchain @bazel_skylib//toolchains/unittest:toolchain_type
  • This is then remapped (in SkylarkRuleClassFunctions.collectToolchainLabels) to //toolchains/unittest:toolchain_type
• The WORKSPACE registers toolchains as @bazel_skylib//toolchains/unittest:bash_toolchain.
  • This is not a label, it is a target pattern
• In RegisteredToolchainsFunction these target patterns are expended via TargetPatternUtil.expandTargetPatterns
  • TargetPatternUtil calls into TargetPatternFunction
  • I have no idea what TargetPatternFunction is doing
  • When the labels are returned to RegisteredToolchainsFunction they have not been remapped and are still of the form @bazel_skylib//toolchains/unittest:bash_toolchain instead of the expected //toolchains/unittest:bash_toolchain

[jonathan-enf]

The fix was rolled back, and this still seems broken in bazel 4.1.0. This bug should be re-opened.

[katre]

Re-opening this but reducing the priority to reflect that no one is actively working on this.

[github-actions]

Thank you for contributing to the Bazel repository! This issue has been marked as stale since it has not had any activity in the last 1+ years. It will be closed in the next 14 days unless any other activity occurs or one of the following labels is added: "not stale", "awaiting-bazeler". Please reach out to the triage team (@bazelbuild/triage) if you think this issue is still relevant or you are interested in getting the issue resolved.

[fmeum]

@katre Pretty sure this has been fixed as otherwise toolchains wouldn't work with Bzlmod.

[katre]

You are correct.