Register apple toolchains by comius · Pull Request #1106 · bazelbuild/rules_swift

comius · 2023-09-08T07:56:51Z

comius · 2023-09-08T07:58:44Z

cc @meteorcloudy @Wyverald Is this the right way to register toolchains?

They are already registered in apple_support repository. Without registering them here the build fails. It might also be the case that registering them hides an underlying bug. (Also note there is a platform_mapping with @@ in use for bzlmod)

meteorcloudy · 2023-09-08T09:21:10Z

Without registering them here the build fails.

How did it fail? Is there a log or an error message?

meteorcloudy · 2023-09-08T09:21:45Z

Oh, it's in #1103?

comius · 2023-09-08T09:23:55Z

Without registering them here the build fails.

How did it fail? Is there a log or an error message?

bazel build --incompatible_enable_cc_toolchain_resolution --enable_bzlmod --platform_mappings=platform_mappings_bzlmod examples/xplatform/hello_world

ERROR: /private/var/tmp/_bazel_ilist/9cf692febd10e3f926566148e9a74eb8/external/_main~non_module_deps~build_bazel_rules_swift_local_config/BUILD:9:22: in xcode_swift_toolchain rule @_main~non_module_deps~build_bazel_rules_swift_local_config//:toolchain: 

	File "/Users/ilist/src/rules_swift/swift/internal/xcode_swift_toolchain.bzl", line 549, column 29, in _xcode_swift_toolchain_impl
		target_triples.parse(cc_toolchain.target_gnu_system_name),
	File "/Users/ilist/src/rules_swift/swift/internal/target_triples.bzl", line 135, column 28, in _parse
		vendor = components[1],
Error: index out of range (index is 1, but sequence has 1 elements)

It looks like this means it's picking up unexpected toolchains. It might have to do with order of registrations.

meteorcloudy · 2023-09-08T09:28:29Z

I thought you mentioned platform_mappings doesn't work with Bzlmod, is that related? (Getting fixed at bazelbuild/bazel#17133)

The order of the toolchain registration is a BFS order from the root module. Can you enable toolchain debug to check what was going on?

meteorcloudy · 2023-09-08T09:30:59Z

Is it possible the toolchain registered from bazel_tools is taking priority? bazel_tools is a default module for all modules.

https://github.com/bazelbuild/bazel/blob/216fce58b81de2c597a78c16514006a4be4891f5/src/MODULE.tools#L13-L15

comius · 2023-09-08T09:34:56Z

Here's output of toolchain_resolution_debug

Working case (register toolchain also in rules_swift):

INFO: ToolchainResolution:   Type @bazel_tools//tools/cpp:toolchain_type: target platform @apple_support~1.9.0//platforms:darwin_arm64: execution @apple_support~1.9.0//platforms:macos_x86_64: Selected toolchain @apple_support~1.9.0~apple_cc_configure_extension~local_config_apple_cc//:cc-compiler-darwin_arm64
INFO: ToolchainResolution:   Type @bazel_tools//tools/cpp:toolchain_type: target platform @apple_support~1.9.0//platforms:darwin_x86_64: execution @apple_support~1.9.0//platforms:macos_x86_64: Selected toolchain @apple_support~1.9.0~apple_cc_configure_extension~local_config_apple_cc//:cc-compiler-darwin_x86_64
INFO: ToolchainResolution:   Type @bazel_tools//tools/cpp:toolchain_type: target platform @apple_support~1.9.0//platforms:darwin_x86_64: execution @apple_support~1.9.0//platforms:macos_x86_64: Selected toolchain @apple_support~1.9.0~apple_cc_configure_extension~local_config_apple_cc//:cc-compiler-darwin_x86_64
INFO: ToolchainResolution:   Type @bazel_tools//tools/cpp:toolchain_type: target platform @apple_support~1.9.0//platforms:darwin_x86_64: execution @apple_support~1.9.0//platforms:macos_x86_64: Selected toolchain @apple_support~1.9.0~apple_cc_configure_extension~local_config_apple_cc//:cc-compiler-darwin_x86_64
INFO: ToolchainResolution:   Type @bazel_tools//tools/cpp:toolchain_type: target platform @apple_support~1.9.0//platforms:darwin_arm64: execution @apple_support~1.9.0//platforms:macos_x86_64: Selected toolchain @apple_support~1.9.0~apple_cc_configure_extension~local_config_apple_cc//:cc-compiler-darwin_arm64

Failing case (register toolchain only in apple_support):

INFO: ToolchainResolution:   Type @bazel_tools//tools/cpp:toolchain_type: target platform @apple_support~1.9.0//platforms:darwin_arm64: execution @apple_support~1.9.0//platforms:macos_x86_64: Selected toolchain @apple_support~1.9.0~apple_cc_configure_extension~local_config_apple_cc//:cc-compiler-darwin_arm64
INFO: ToolchainResolution:   Type @bazel_tools//tools/cpp:toolchain_type: target platform @apple_support~1.9.0//platforms:darwin_x86_64: execution @apple_support~1.9.0//platforms:macos_x86_64: Selected toolchain @bazel_tools~cc_configure_extension~local_config_cc//:cc-compiler-darwin_x86_64
INFO: ToolchainResolution:   Type @bazel_tools//tools/cpp:toolchain_type: target platform @apple_support~1.9.0//platforms:darwin_x86_64: execution @apple_support~1.9.0//platforms:macos_x86_64: Selected toolchain @bazel_tools~cc_configure_extension~local_config_cc//:cc-compiler-darwin_x86_64
INFO: ToolchainResolution:   Type @bazel_tools//tools/cpp:toolchain_type: target platform @apple_support~1.9.0//platforms:darwin_x86_64: execution @apple_support~1.9.0//platforms:macos_x86_64: Selected toolchain @bazel_tools~cc_configure_extension~local_config_cc//:cc-compiler-darwin_x86_64
INFO: ToolchainResolution:   Type @bazel_tools//tools/cpp:toolchain_type: target platform @apple_support~1.9.0//platforms:darwin_arm64: execution @apple_support~1.9.0//platforms:macos_x86_64: Selected toolchain @apple_support~1.9.0~apple_cc_configure_extension~local_config_apple_cc//:cc-compiler-darwin_arm64

Diff is in middle 3 labels: @apple_support~1.9.0~apple_cc_configure_extension~local_config_apple_cc -> @bazel_tools~cc_configure_extension~local_config_cc

Meaning that Bazel supplied C++ toolchains were selected instead of apple_support ones.

comius · 2023-09-08T09:38:04Z

The fix is probably to consolidate C++ toolchain configuration in Apple Support with the ones in bazel_tools / rules_cc. Maybe also make rules_swift less dependent on the strings in C++ toolchains.

@keith can you look into this?

meteorcloudy · 2023-09-08T09:41:10Z

We can probably try to inject bazel_tools as the last bazel module instead of the first one. /cc @Wyverald

But yes, in anyway, rules_swift shouldn't fail with Error: index out of range during a mismatch.

comius · 2023-09-08T09:54:47Z

The order of the toolchain registration is a BFS order from the root module. Can you enable toolchain debug to check what was going on?

@meteorcloudy:
Does BFS means here: root-module, @bazel_tools, @apple_support? Can it sometimes happen its: @bazel_tools, root-module, @apple_supporT?

comius · 2023-09-08T09:57:32Z

cc @katre

At risk of developing a specification in a comment, I think the order should be BFS and then @bazel_tools. Within BFS, the order in MODULE.bazel should be respected, that is the order of deps inside.

meteorcloudy · 2023-09-08T10:11:13Z

Does BFS means here: root-module, @bazel_tools, @apple_support?

Yes, I guess a better order would be root-module, <bazel_dep in root module's MODULE.bazel> then bazel_tools, then rest modules in second+ layers.

Can it sometimes happen its: @bazel_tools, root-module, @apple_supporT?

No, the toolchain defined in root-module will definitely prevail.

katre · 2023-09-08T15:28:21Z

This seems related to an internal issue about making toolchain priority more flexible (bug 262537944 for googlers). I'd rather address that, and mark @bazel_tools as the lowest priority, before adding bzlmod-specific ordering behavior.

keith · 2023-09-08T17:36:28Z

definitely seems worth a fix, having to make users do this duplicate registration seems unexpected

This will ensure toolchains registered from bazel_dep in users's MODULE.bazel take priority over the ones registered in `bazel_tools`. Context: bazelbuild/rules_swift#1106 RELNOTES: None PiperOrigin-RevId: 563702381 Change-Id: I0b207cb24f34c1d2906123787216bba59ce5d442

- This will ensure toolchains registered from bazel_dep in users's MODULE.bazel take priority over the ones registered in `bazel_tools`. - If the root module already has an override for a builtin module, then the default builtin module override is ignored. Context: bazelbuild/rules_swift#1106 RELNOTES: None PiperOrigin-RevId: 566969573 Change-Id: I0b207cb24f34c1d2906123787216bba59ce5d442

…19573) - This will ensure toolchains registered from bazel_dep in users's MODULE.bazel take priority over the ones registered in `bazel_tools`. - If the root module already has an override for a builtin module, then the default builtin module override is ignored. Context: bazelbuild/rules_swift#1106 RELNOTES: None PiperOrigin-RevId: 566969573 Change-Id: I0b207cb24f34c1d2906123787216bba59ce5d442

Regsiter apple toolchains

a5d99d4

comius requested review from BalestraPatrick, brentleyjones, keith, mattrobmattrob, segiddins and thii as code owners September 8, 2023 07:56

comius marked this pull request as draft September 8, 2023 07:57

comius mentioned this pull request Sep 8, 2023

--incompatible_enable_cc_toolchain_resolution + universal tools fails to build #1103

Closed

meteorcloudy mentioned this pull request Sep 19, 2023

Flip --incompatible_enable_cc_toolchain_resolution bazelbuild/bazel#19441

Closed

meteorcloudy mentioned this pull request Sep 20, 2023

[6.4.0] Inject builtin modules at the end of the MODULE.bazel file bazelbuild/bazel#19573

Merged

brentleyjones closed this Oct 18, 2023

Conversation

comius commented Sep 8, 2023

Uh oh!

comius commented Sep 8, 2023

Uh oh!

meteorcloudy commented Sep 8, 2023

Uh oh!

meteorcloudy commented Sep 8, 2023

Uh oh!

comius commented Sep 8, 2023

Uh oh!

meteorcloudy commented Sep 8, 2023

Uh oh!

meteorcloudy commented Sep 8, 2023

Uh oh!

comius commented Sep 8, 2023

Uh oh!

comius commented Sep 8, 2023

Uh oh!

meteorcloudy commented Sep 8, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

comius commented Sep 8, 2023

Uh oh!

comius commented Sep 8, 2023

Uh oh!

meteorcloudy commented Sep 8, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

katre commented Sep 8, 2023

Uh oh!

keith commented Sep 8, 2023

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

meteorcloudy commented Sep 8, 2023 •

edited

Loading

meteorcloudy commented Sep 8, 2023 •

edited

Loading