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.

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

publicly visible Bazel target no longer provides any headers #596

Closed
shahms opened this issue Oct 28, 2020 · 5 comments · Fixed by #615
Closed

publicly visible Bazel target no longer provides any headers #596

shahms opened this issue Oct 28, 2020 · 5 comments · Fixed by #615
Milestone

Comments

@shahms
Copy link

shahms commented Oct 28, 2020

The only target accessible via Bazel (@com_github_google_glog//:glog) no longer provides any headers, due to PR ##488. This breaks when using features = ["layering_check"]. Given the low-level nature of logging libraries and pervasive dependencies in a code base which uses them, it's infeasible to disable that feature for just the targets which depend on glog. The benefits afford by the check (particularly for code bases which are imported internally) means that disabling it for our entire code base isn't reasonable.

drigz added a commit to drigz/glog that referenced this issue Mar 1, 2021
@drigz
Copy link
Member

drigz commented Mar 1, 2021

Do you have a repro for this? #615 seems not to break the build, but I wasn't able to find docs on how to enable layering_check.

@shahms
Copy link
Author

shahms commented Mar 1, 2021

You can enabled layering_check by adding:

package(features = ["layering_check"]) to a BUILD file or features = ["laying_check"] to the specific cc_ rules in question.

@drigz
Copy link
Member

drigz commented Mar 1, 2021

I tried that but it didn't reproduce the failure with glog at HEAD.

@shahms
Copy link
Author

shahms commented Mar 1, 2021

I forgot that we use Clang to do the build. Bazel uses GCC by default, which doesn't support layering_check. Building with clang, via:

build build --client_env=CC=clang  ...

will reproduce

drigz added a commit to drigz/glog that referenced this issue Mar 2, 2021
This will avoid regressions on google#596.
drigz added a commit to drigz/glog that referenced this issue Mar 2, 2021
drigz added a commit to drigz/glog that referenced this issue Mar 2, 2021
This will avoid regressions on google#596.
@drigz
Copy link
Member

drigz commented Mar 2, 2021

Thank you! I was able to reproduce it, and #615 seems to fix it.

@drigz drigz closed this as completed in #615 Mar 2, 2021
drigz added a commit that referenced this issue Mar 2, 2021
drigz added a commit that referenced this issue Mar 2, 2021
This will avoid regressions on #596.
@sergiud sergiud added this to the 0.5 milestone Mar 30, 2021
@sergiud sergiud mentioned this issue May 6, 2021
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 a pull request may close this issue.

3 participants