rules_swift icon indicating copy to clipboard operation
rules_swift copied to clipboard

Configure LLVM cc_toolchain included with Swift releases

Open gferon opened this issue 2 years ago • 4 comments

On Linux, and to some extent on macOS, Swift ships with its own version of clang.

Our problem: we have a tree with a lot of targets for different platforms, and we need to make sure that Swift built binaries are using the Apple-shipped clang if we decide to configure another default C++ toolchain for the rest of the build.

This is currently not possible, because rules_swift assumes the use of @bazel_tools//tools/cpp:toolchain_type for its cc_toolchain.

  • On macOS, this PR is essentially doing nothing, as we can alias @local_config_apple_cc//:toolchain
  • On Linux, it's a little bit more involved, as you need to run CC toolchain detection on the clang provided by Apple from https://www.swift.org/download/. Fortunately, the relevant functions are part of the public API of @bazel_tools.
  • On Windows, we do the same as on Linux, except we use the Windows functions from @bazel_tools.

This means that we can remove the check for CC=clang entirely and leave this choice to the user (which they might need if they have native code that only compiles with clang for example). It is also a prerequisite for downloading Swift toolchains (except on macOS) directly in Bazel, which should now be possible.

gferon avatar Sep 18 '23 15:09 gferon

I think the general assumption today to solve this is by doing CC=clang in the env when you exec bazel that the clang in that statement is the one from Swift, so the standard configuration picks up the tools that were installed with Swift instead of the system tools (that might not apply for llvm-ar etc unless those are symlinked to just ar)

keith avatar Sep 18 '23 17:09 keith

I think the general assumption today to solve this is by doing CC=clang

Yes, and this is the problem I'm trying to solve. You want to make sure rules_swift picks up those tools, but not necessarily the rest of the build. With this patch, you can stop doing this.

There's even a TODO here (that I forgot to remove in this PR) https://github.com/bazelbuild/rules_swift/blob/953c2815ebacacbf63bcda10c2d776dd03f55267/swift/internal/swift_autoconfiguration.bzl#L425 explaining what I'm trying to achieve.

gferon avatar Sep 19 '23 09:09 gferon

@gferon I fixed the merge conflict. Did we want to merge this @keith @thii?

luispadron avatar Nov 17 '23 05:11 luispadron

I took this for a spin and I noticed that right now this works via using the old toolchain setup, which is a side effect of us not migrating to the new way sooner. Theoretically we should replace the _cc_toolchain attribute with https://github.com/bazelbuild/rules_apple/blob/c74704ac9a41c6998191d26bf418152bda9d2f26/apple/cc_toolchain_forwarder.bzl#L134, which results in this no longer working correctly. Similar on the consuming side instead of doing ctx.attr._cc_toolchain we should be using https://github.com/bazelbuild/rules_apple/blob/c74704ac9a41c6998191d26bf418152bda9d2f26/apple/internal/watchos_rules.bzl#L158

If we make these changes this no longer works because the newly created toolchains here aren't registered. I think the ideal solution would be that we would create these toolchains as you are now, register them with the normal infra, and then bazel would discover them correctly. The potential problem with that is I think we need another constraint of some sort on the toolchain to make sure bazel picks the right one in that case, and I'm not sure if there's a way to do that while still calling configure_unix_toolchain.

I noticed this because on macOS this change is actually a no-op for the wrong reasons, since we're using the new API already https://github.com/bazelbuild/rules_swift/blob/a0af0c0fd3c784198fe66780cf4de3157bfb1462/swift/internal/xcode_swift_toolchain.bzl#L555 and the result is the default CC toolchain is chosen instead of the one being aliased and specified here

keith avatar Jan 18 '24 18:01 keith