search

bazel-contrib / toolchains_llvm

LLVM toolchain for bazel
Apache License 2.0
289 stars 207 forks source link

Replace usage of `tool_paths` with `action_configs` #293

Closed sluongng closed 5 months ago

sluongng commented 5 months ago

tool_paths is considered deprecated. https://github.com/bazelbuild/bazel/issues/8438#issuecomment-1523519395

We should switch to use action_configs instead.

siddharthab commented 5 months ago

But bazel's cc_toolchain_config, which we wrap around, still relies on it and does not take in action_configs. https://cs.opensource.google/bazel/bazel/+/refs/tags/7.0.0:tools/cpp/unix_cc_toolchain_config.bzl;l=182-185

If we did not wrap around it, maybe we could skip using it. In the actual Java code, it is still being passed on deeper, but its direct use is gated by "no_legacy_features". https://cs.opensource.google/bazel/bazel/+/master:src/main/java/com/google/devtools/build/lib/rules/cpp/CcModule.java;l=1257;drc=6743d76f9ecde726d592e88d8914b9db007b1c43

siddharthab commented 5 months ago

Also, this function still relies on tool paths, even with "no_legacy_features": https://cs.opensource.google/bazel/bazel/+/master:src/main/starlark/builtins_bzl/common/cc/cc_toolchain_provider_helper.bzl;l=75;drc=f0150efd1cca473640269caaf92b5a23c288089d

jsharpe commented 5 months ago

There's also a number of fields in the CCToolchainInfo e.g. ld_executable that only gets populated correctly when using tool paths; this breaks usage of some rulesets if you switch to an action_config only cc toolchain at the moment.

sluongng commented 5 months ago

@jsharpe I think the intention is to eventually deprecate those APIs in favor of cc_common.get_tool_for_action as stated in the linked thread.

It seems like the migration is still on-going from Bazel side though. So let's revisit this around Bazel 8 to see if things changed.

jsharpe commented 5 months ago

@sluongng Yep I'm aware of that but I think get_tool_for_action is missing equivalents for some of the tools (specifically at least ld) that are currently exposed; I ran into this in rules_foreign_cc - see https://github.com/bazelbuild/rules_foreign_cc/issues/1160