Skip to content

Conversation

@keith
Copy link
Member

@keith keith commented Nov 27, 2025

@keith
Copy link
Member Author

keith commented Nov 27, 2025

cc @trybka

Copy link
Collaborator

@armandomontanez armandomontanez left a 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.

@armandomontanez
Copy link
Collaborator

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.

@keith
Copy link
Member Author

keith commented Dec 19, 2025

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 #include if the textual_hdrs match the strip_include_prefix? (I thought you would still be able to access them through the fully path 🤔 )

@trybka
Copy link
Collaborator

trybka commented Dec 19, 2025

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).

@keith keith force-pushed the ks/make-strip_include_prefix-apply-to-textual_hdrs branch from 6e30a20 to 72348e4 Compare December 19, 2025 21:06
@keith
Copy link
Member Author

keith commented Dec 19, 2025

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)

@trybka
Copy link
Collaborator

trybka commented Dec 23, 2025

(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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants