-
Notifications
You must be signed in to change notification settings - Fork 144
Support for injecting custom toolchain features (approach injecting rule-based features with example) #524
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
base: main
Are you sure you want to change the base?
Conversation
This is a draft proposal to inject rules-based toolchain features into the legacy toolchain.
f8959db to
4295909
Compare
fmeum
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Not a maintainer, but this is a slick path for incremental migrations - looks great and would also help toolchains_llvm.
cc @keith
|
yea i think something like this would be great. i wouldn't be surprised if there was a bit of pushback on using the new API in the older style toolchains |
armandomontanez
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I like this idea. While this isn't necessarily an ideal state for anyone to live in long term, I think it's still overall a net positive.
| @@ -0,0 +1,6 @@ | |||
| load("@rules_cc//cc:cc_binary.bzl", "cc_binary") | |||
|
|
|||
| cc_binary( | |||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Was this intended to be built with the toolchain in //examples/inject_extra_features/toolchain? The toolchain isn't registered.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@armandomontanez it was intended that it is not register. For the moment I just wanted to add an example in the draft in this way people could play around with the command line that I mentioned in the PR description.
If between #523 and #524 you also agree that #524 is the preferred approach, then I would just close both of them and we can continue the discussion in #525. You will see that in #525 I did not add any example, I believe that it is quite simple as to need an example, but if you want one let me know.
Hi, this is just a draft how a proposal could look like to implement #517 on the legacy toolchain approach but injecting features defined with the new rules-based toolchain approach. In my opinion it looks much more elegant than #523 and it would also be easier for users to later migrate to the new rules-based toolchain approach.
Let me know your opinion
If you want to try it out just run:
bazel build //examples/inject_extra_features:main --extra_toolchains=//examples/inject_extra_features/toolchain:my_linux_toolchain --toolchain_resolution_debug=.* --subcommands=pretty_print --features=foo