Skip to content

Fix system_include_paths with textual_hdrs/hdrs and strip_include_prefix - #800

Open
keith wants to merge 2 commits into
bazelbuild:mainfrom
keith:ks/fix-system_include_paths-with-textual_hdrs
Open

Fix system_include_paths with textual_hdrs/hdrs and strip_include_prefix#800
keith wants to merge 2 commits into
bazelbuild:mainfrom
keith:ks/fix-system_include_paths-with-textual_hdrs

Conversation

@keith

@keith keith commented Jul 20, 2026

Copy link
Copy Markdown
Member

From #537

trybka
trybka previously approved these changes Jul 20, 2026
Comment on lines 496 to 500
if public_headers.virtual_include_path:
if external:
external_include_dirs.append(public_headers.virtual_include_path)
else:
include_dirs_for_context.append(public_headers.virtual_include_path)

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

If it is deliberate that this does not match, could you add a comment as to why?

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

nice spot, i added it on this one too

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

FWIW I suggested this same change internally on friday (though without the tests you wrote), and am working through some internal build failures.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

ah cool. if that lands I can rebase for just the tests

@keith keith changed the title Fix system_include_paths with textual_hdrs Fix system_include_paths with textual_hdrs/hdrs and strip_include_prefix Jul 20, 2026
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