Make strip_include_prefix apply to textual_hdrs

[keith]

Fixes #12424

This reverts commit dfc72d0.

[keith]

new version of #25749

[keith]

@pzembrod ptal for a reland 🙏🏻 hopefully the solution of allowing textual_hdrs to not start with strip_include_prefix is acceptable. I don't see another solution that won't break existing code. I can squash these commits once you review

[Copilot]

Pull Request Overview

This PR reverts a previous change to re-enable applying strip_include_prefix to textual_hdrs. It adds a test case and updates the helper logic to handle textual headers similarly to public headers.

• Add textual_hdrs test files and BUILD rule
• Extend _compute_public_headers signature and logic for non-module headers
• Integrate textual_headers into compilation context (include paths, coverage mapping, declared inputs)

Reviewed Changes

Copilot reviewed 6 out of 6 changed files in this pull request and generated 1 comment.

Show a summary per file

File	Description
src/main/starlark/tests/.../test_include_textual.cc	New C++ test covering strip_include_prefix on textual headers
src/main/starlark/tests/.../not_nested.h	New textual header outside nested dir
src/main/starlark/tests/.../nested/lib.h	New nested header
src/main/starlark/tests/.../nested/lib.cc	Implementation of foo() and its include
src/main/starlark/tests/.../BUILD.builtin_test	Add cc_library/cc_test with strip_include_prefix and textual_hdrs
src/main/starlark/builtins_bzl/common/cc/compile/cc_compilation_helper.bzl	Update _compute_public_headers signature and logic; wire textual headers into context

Comments suppressed due to low confidence (1)

src/main/starlark/tests/builtins_bzl/cc/basics/test/nested/lib.cc:14

• When using strip_include_prefix = "nested", the header should be included as #include "lib.h" (or #include "nested/lib.h" before stripping) instead of the full workspace path to match the virtual include layout.

#include "src/main/starlark/tests/builtins_bzl/cc/basics/test/nested/lib.h"

[pzembrod]

@pzembrod ptal for a reland 🙏🏻 hopefully the solution of allowing textual_hdrs to not start with strip_include_prefix is acceptable. I don't see another solution that won't break existing code. I can squash these commits once you review

Yes, your reasoning makes sense to me. And I'd actually be okay with leaving the two commits; once they get piped through the import and export mechanism, they will turn into one commit in the end anyway, I think.

[keith]

ok great, any thoughts on #25749 (comment)

[iancha1992]

@bazel-io fork 8.3.0

[keith]

this was reverted again in dfc72d0

[keith]

I pushed the fix here 6e4698b (#26327) this seems to break internally at google only, so I don't have a good test case for it

[trybka]

LGTM. Tested with some of the internal breakages I was tracking. The added test failed before these fixes, and passes now.

[keith]

@iancha1992 hit any blockers?

[iancha1992]

@iancha1992 hit any blockers?

Hi @keith
@trybka is actually importing this one

[trybka]

Can you rebase this?

I suspect some changes may need to be made after some changes to introduce _ModuleMapInfo provider.

Currently getting :

...
        File "/virtual_builtins_bzl/common/cc/cc_common.bzl", line 673, column 19, in _compile
        File "/virtual_builtins_bzl/common/cc/compile/compile.bzl", line 245, column 112, in compile
        File "/virtual_builtins_bzl/common/cc/compile/cc_compilation_helper.bzl", line 526, column 38, in _init_cc_compilation_context
        File "/virtual_builtins_bzl/common/cc/compile/cc_compilation_helper.bzl", line 325, column 33, in _create_module_map_action
        File "/virtual_builtins_bzl/common/cc/compile/cc_compilation_helper.bzl", line 189, column 26, in _ModuleMapInfo
Error: got unexpected field 'textual_headers' in call to instantiate provider _ModuleMapInfo

[keith]

fixed, looks like it was just that thing needing a new field

[keith]

rebased to fix the conflict and add missing rules_cc imports, AFAIUI the tests are only failing because of how this has to mirrored to rules_cc at the same time as the import

[trybka]

is there a separate change for rules_cc as well?

[keith]

No but I can make one if that is easier

[keith]

created bazelbuild/rules_cc#537 if that's easier

[trybka]

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.

Are the buildkite failures something we can address in this change, or are they just related to rules_cc version skew?

[keith]

Yea it's that it's reading the rules_cc one for the tests which isn't sync'd.