Use relative labels and Label macro wherever possible
#543
Conversation
|
This PR came from some exploration i was doing in trying to support either agnostic workspace names for this repository or at least both |
| executable = True, | ||
| allow_single_file = True, | ||
| cfg = "exec", | ||
| ), | ||
| }, | ||
| outputs = {"out": "%{name}.rs"}, | ||
| toolchains = [ | ||
| "@io_bazel_rules_rust//bindgen:bindgen_toolchain", | ||
| "@io_bazel_rules_rust//rust:toolchain", | ||
| str(Label("//bindgen:bindgen_toolchain")), |
There was a problem hiding this comment.
I'm shopping around for some thoughts here and generally more insight on Label. I'm not sure if this is an appropriate use of Label. Anyone have any experiences like this they can share?
Specifically around str(Label("..."))
There was a problem hiding this comment.
This is perfectly acceptable. I remember having a funny discussion with several senior engineer in the Bazel team about this "hack" and having the manager answer: I don't think this is a hack, which just settled our decision to keep this as an idiom, although I left the team 3 years ago so I don't know what is the status now.
|
@dfreese Hey, are you able to take a look at this 🙏 |
|
@damienmg hey, you wouldn't als have time to look at this would you 😅 🙏 |
This allows for the workspace to have an arbitrary name with the exception of any thing that was output by `cargo-raze`
|
@illicitonion Hey, would you have time to take a look at this? 🙏 😅 |
|
@damienmg Hey, spoke with @illicitonion and he thinks you'd be the best reviewer for this change. Would you have a quick moment to take a look at this this week? |
| executable = True, | ||
| allow_single_file = True, | ||
| cfg = "exec", | ||
| ), | ||
| }, | ||
| outputs = {"out": "%{name}.rs"}, | ||
| toolchains = [ | ||
| "@io_bazel_rules_rust//bindgen:bindgen_toolchain", | ||
| "@io_bazel_rules_rust//rust:toolchain", | ||
| str(Label("//bindgen:bindgen_toolchain")), |
There was a problem hiding this comment.
This is perfectly acceptable. I remember having a funny discussion with several senior engineer in the Bazel team about this "hack" and having the manager answer: I don't think this is a hack, which just settled our decision to keep this as an idiom, although I left the team 3 years ago so I don't know what is the status now.
|
This is the right approach, this repository was created before we made the label behavior consistent and thus was using aboslute label approach, but there is no reason to do this anymore. |
|
@damienmg Thanks for the review, that bit of context, and updating the branch 🙏 CI seems to still be passing 😄 |
This PR updates changes the explicit use of
@io_bazel_rules_rust//...to//...and adds the use of Label wherever possible. This should currently have no functional but it provides some better type guarantees.