google / benchmark

A microbenchmark support library
Apache License 2.0
8.91k stars 1.62k forks source link

[FR] cc_libraries should use "includes", not "strip_include_prefix" #1512

Open carlosgalvezp opened 1 year ago

carlosgalvezp commented 1 year ago

So that when users import google-benchmark, the headers are included as "system headers", instead of regular headers.

If this is not done, compilers will throw warnings at google-benchmark code, which users have no way to act upon. Users should be able to choose a highly restrictive set of warnings without having to fix them in google-benchmark.

Note that this pattern is already applied in the googletest repo.

varshneydevansh commented 1 year ago

Is the below discussion related to the above issue?

Bazel "strip_include_prefix" or "includes" attributes in cc_import rules suggested solution in this thread -

  1. https://github.com/bazelbuild/bazel/issues/4748#issuecomment-515887921
  2. https://github.com/bazelbuild/bazel/issues/4748#issuecomment-1260781555

I would like to work on this after completing my pending PR.

I went on reading what strip_include_prefix is?

carlosgalvezp commented 1 year ago

Hi! No, those links are related to cc_import, while I'm referring to cc_library in this ticket :)

varshneydevansh commented 1 year ago

Yes that bazel issue is about cc_import but the workaround for that and with the cc_library they used includes instead of strip_include_prefix. So, I found it a good starting point.

I am looking more into this. https://groups.google.com/g/bazel-discuss/c/pQLmBVNJGZ4?pli=1

googletest BUILD.bazel