Skip to content
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

strip_include_prefix doesn't apply to textual_hdrs #12424

Open
chandlerc opened this issue Nov 6, 2020 · 2 comments
Open

strip_include_prefix doesn't apply to textual_hdrs #12424

chandlerc opened this issue Nov 6, 2020 · 2 comments
Assignees
@lberki
Labels
P4 This is either out of scope or we don't have bandwidth to review a PR. (No assignee) team-Rules-CPP Issues for C++ rules type: bug

Comments

@chandlerc
Copy link

Description of the problem / feature request:

strip_include_prefix in cc_* rules is super useful, but it needs to apply to textual_hdrs as well as hdrs. The inconsistency is extremely problematic for rules successfully using either textual_hdrs or strip_include_prefix.

Bugs: what's the simplest, easiest way to reproduce this bug? Please provide a minimal example if possible.

Given this rule:

cc_library(
    name = "test",
    hdrs = ["some/path/prefix/normal_header.h"
    textual_hdrs = ["some/path/prefix/textual_header.h"],
    strip_include_prefix = "some/path/prefix",
)

Rules depending on :test can #include "normal_header.h" as expected, but cannot #include "textual_header.h".

What operating system are you running Bazel on?

Impacts all OSes AFAICT, tested on Linux.

What's the output of bazel info release?

release 3.3.1
@hlopko hlopko added the team-Rules-CPP Issues for C++ rules label Nov 6, 2020
@chandlerc chandlerc mentioned this issue Nov 6, 2020
Merged
chandlerc added a commit to google/llvm-bazel that referenced this issue Nov 6, 2020
@chandlerc
Remove the includes directive from tblgen libraries. (#82)
e982199 
This caused every single directory tree with a tblgen output in it to be
in the `-isystem` header search path. That is far from desirable, and
was hiding a large number of issues reaching both generated files and
source files with `#include`.

On MacOS it also manifested in an amazing problem where a header in
Clang's AST library called `Availability.h` ended up being used instead
of an internal header by the same name inside of the MacOS libc to
provide system availability macros to the libc headers. The resulting
errors were... fun.

To get these generated files to be found, this change adds
`strip_include_prefix` where necessary to make the relevant headers
available at the expected paths. This is much more narrow and focused.
Making it fully work requires two other workarounds. First, we need to
introduce extra globs or libraries to provide the same headers through
different include paths in a few places that were being hidden by the
`includes` from `tblgen`. Second, we have to switch to `hdrs` instead of
`textual_hdrs` in numerous places to work around a Bazel bug:
bazelbuild/bazel#12424

However, with this, we get the Bazel side to a clean, principled
implementation of the requirements of LLVM.
@oquenchil oquenchil added P4 This is either out of scope or we don't have bandwidth to review a PR. (No assignee) and removed untriaged labels Nov 19, 2020
@GMNGeoffrey GMNGeoffrey mentioned this issue Aug 17, 2021
Open
@github-actions
Copy link

Thank you for contributing to the Bazel repository! This issue has been marked as stale since it has not had any activity in the last 2+ years. It will be closed in the next 14 days unless any other activity occurs or one of the following labels is added: "not stale", "awaiting-bazeler". Please reach out to the triage team (@bazelbuild/triage) if you think this issue is still relevant or you are interested in getting the issue resolved.

@github-actions github-actions bot added the stale Issues or PRs that are stale (no activity for 30 days) label Apr 13, 2023
@chandlerc
Copy link
Author

@bazelbuild/triage - This is definitely still relevant and I'd like to see it resolved.

Pretty sure it was triaged and a team added back in 2020, but then marked as untriaged... 🤷

@sgowroji sgowroji removed the stale Issues or PRs that are stale (no activity for 30 days) label Apr 13, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@lberki lberki

Labels
P4 This is either out of scope or we don't have bandwidth to review a PR. (No assignee) team-Rules-CPP Issues for C++ rules type: bug
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
6 participants
@philwo @chandlerc @hlopko @lberki @oquenchil @sgowroji