Support for injecting custom toolchain features
There are situations where it is helpful to implement a toolchain feature for all toolchains that your project supports. In our case, this was used for implementing different warning levels/groups that is compiler agnostic. That allows us to have something like:
cc_library(
name = "foo",
srcs = ["foo.cpp"],
hdrs = ["foo.h"],
features = [
"safety_related_warnings",
],
)
cc_library(
name = "bar",
srcs = ["bar.cpp"],
hdrs = ["bar.h"],
features = [
"security_related_warnings",
],
)
The main question for the maintainers would be, are you interested in that feature? Do you see this as something that we could add to toolchains_llvm? If yes just add a comment here and I would start working right away.
The short answer is yes, we absolutely want this, but the proper way to achieve that would probably be by porting toolchains_llvm over to the new rules-based C++ toolchain approach.
CC @thesayyn @keith @dzbarsky for their thoughts
Understood. Let me know if a contribution could be made before toolchains_llvm fully switches to new rules-based C++ toolchain approach or not. I assume it would take some time to be fully on the new approach.
That's true and I don't want to let perfect be the enemy of good here. If we find a way to make this possible that has a clear migration path, we could also decide to do it now.
Ok, I will explore a bit and see what is best. If there is any work in progress for the new way that is not on master let me know. I could also check if I find time to contribute there.
I definitely think the rules based toolchain can provide a much better API for doing something like this, which I'm very interested in. I haven't attempted to make that work yet though, there are still some subtle issues we have to worry about like ordering vs the defaults, but should be nicer with that approach for sure
I took a first look and the problem at least without rules-based toolchains is that toolchains_llvm depends on cc_toolchain_config from rules_cc and that does not support injecting features.
https://github.com/bazel-contrib/toolchains_llvm/blob/19dc6cf50de936407198f19e9ffc1af90aef01a7/toolchain/cc_toolchain_config.bzl#L17
https://github.com/bazelbuild/rules_cc/blob/b5a65591334f74371f4d75003768957a740cd868/cc/private/toolchain/unix_cc_toolchain_config.bzl#L1954
Even though the underlying create_cc_toolchain_config_info does support it
https://github.com/bazelbuild/rules_cc/blob/b5a65591334f74371f4d75003768957a740cd868/cc/private/toolchain/unix_cc_toolchain_config.bzl#L1936
This is inside private in rules_cc but I assume that this was agreed with rules_cc authors. To me it makes sense that specific toolchains can make use of unix_cc_toolchain_config.bzl that is nothing else than a configurable template.
I think that whatever it is decided, needs to be agreed with rules_cc maintainers. Because supporting injecting of features without support from rules_cc I do not think it is worth it, otherwise other C++ toolchains would have the same problem.
I will create a ticket in rules_cc to discuss this topic.
Here is a draft on how the injection in rules_cc for legacy toolchains could look like. If anyone here has an opinion please just comment on it. In the end toolchains_llvm would have to use such API.
https://github.com/bazelbuild/rules_cc/pull/523