-
Notifications
You must be signed in to change notification settings - Fork 145
Make strip_include_prefix apply to textual_hdrs #537
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?
Make strip_include_prefix apply to textual_hdrs #537
Conversation
|
cc @trybka |
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.
Will probably take a bit to get this one merged.
|
Still seeing failures internally, we might need to put this behind a flag if we don't want to block on Whatever Problems Google Has. |
|
are you thinking a top level bazel flag, or a magic feature to opt out of this behavior, or? also is there a bug in the implementation that only google is seeing? or is it just that maybe you have to update your |
|
I think we could handle it in semantics.bzl rather than a flag or a feature. The former would need to either be in Java or Starlark Build Flags both of which seem pretty onerous, the latter of which would need to be yet-another-feature we need to check and we've already encoutered issues using features for this in the past (notably things like platform.flags dropping them on the floor: https://bazel.build/reference/be/platforms-and-toolchains#platform_flags_repeated). |
6e30a20 to
72348e4
Compare
|
pushed a boolean to that file which can disable this behavior, wdyt about that? idk how your replacements for that file work but I assume it might have the downside that it cannot be enabled in some places and not others (notably the original feature requester of this is a googler) |
|
(Replied to the wrong PR...) The goal would be to enable it internally soon as well, but to be able to approve this change without blocking on said clean-up. (The buildkite failures are only in the other PR) It looks like this is passing most of the builds that were failing before. |
Mirror of bazelbuild/bazel#26327