Skip to content

Fix layering_check with path stripping and textual_hdrs #527

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.

Sign up for GitHub

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

Jump to bottom
Open
wants to merge 1 commit into
base: main
Choose a base branch
from
Open

Fix layering_check with path stripping and textual_hdrs #527

wants to merge 1 commit into from
+12 −2

Conversation

@keith
Copy link
Member

@keith keith commented Nov 19, 2025

Previously textual_hdrs paths were eagerly fetched which breaks path
stripping. This wires these headers up separately to treat them like
hdrs.

Fixes bazelbuild/bazel#27718

@keith
Fix layering_check with path stripping and textual_hdrs
f64afe1 
Previously textual_hdrs paths were eagerly fetched which breaks path
stripping. This wires these headers up separately to treat them like
hdrs.

Fixes bazelbuild/bazel#27718
@keith keith mentioned this pull request Nov 19, 2025
Open
@keith
Copy link
Member Author

keith commented Nov 19, 2025

i chose to split these out from additional_exported_headers because that field is undocumented and publicly unused. If googlers validate it wouldn't break them we could instead change the type of that argument to be a list of Files instead of a list of strings.

keith
keith commented Nov 19, 2025
View reviewed changes
cc/private/compile/cc_compilation_helper.bzl
continue
add_header(path = header.path, visibility = "", can_compile = False)
added_paths.add(header.path)

Copy link
Member Author

@keith keith Nov 19, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

note that the core difference is below the paths are already strings, where here we are processing file objects in a context where their paths can be stripped by bazel

@keith
Copy link
Member Author

keith commented Nov 19, 2025

ptal @fmeum @pzembrod

fmeum
fmeum approved these changes Nov 19, 2025
View reviewed changes
Copy link
Collaborator

@fmeum fmeum left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the fix!

Cc @c-mita as the original author

@fmeum
Copy link
Collaborator

fmeum commented Nov 19, 2025

@bazel-io fork 9.0.0

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

Reviewers

@trybka trybka Awaiting requested review from trybka trybka is a code owner

@armandomontanez armandomontanez Awaiting requested review from armandomontanez armandomontanez is a code owner

@pzembrod pzembrod Awaiting requested review from pzembrod pzembrod is a code owner

@comius comius Awaiting requested review from comius comius is a code owner

@c-mita c-mita Awaiting requested review from c-mita c-mita is a code owner

@hvadehra hvadehra Awaiting requested review from hvadehra hvadehra is a code owner

1 more reviewer

@fmeum fmeum fmeum approved these changes

Reviewers whose approvals may not affect merge requirements

At least 1 approving review is required to merge this pull request.

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

layering_check for CppCompile doesn't work with generated files in textual_hdrs

2 participants

@keith @fmeum