Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Register apple toolchains #1106

Closed
wants to merge 1 commit into from

Conversation

comius
Copy link

@comius comius commented Sep 8, 2023

@comius
Copy link
Author

comius commented Sep 8, 2023

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
Copy link
Member

Without registering them here the build fails.

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

@meteorcloudy
Copy link
Member

Oh, it's in #1103?

@comius
Copy link
Author

comius commented Sep 8, 2023

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
Copy link
Member

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
Copy link
Member

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
Copy link
Author

comius commented Sep 8, 2023

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
Copy link
Author

comius commented Sep 8, 2023

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
Copy link
Member

meteorcloudy commented Sep 8, 2023

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
Copy link
Author

comius commented Sep 8, 2023

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
Copy link
Author

comius commented Sep 8, 2023

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
Copy link
Member

meteorcloudy commented Sep 8, 2023

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
Copy link
Member

katre commented Sep 8, 2023

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
Copy link
Member

keith commented Sep 8, 2023

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

SalmaSamy pushed a commit to bazelbuild/bazel that referenced this pull request Sep 20, 2023
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
copybara-service bot pushed a commit to bazelbuild/bazel that referenced this pull request Sep 20, 2023
- 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
meteorcloudy added a commit to meteorcloudy/bazel that referenced this pull request Sep 20, 2023
- 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
meteorcloudy added a commit to bazelbuild/bazel that referenced this pull request Sep 21, 2023
…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
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

--incompatible_enable_cc_toolchain_resolution + universal tools fails to build
5 participants