google / benchmark

A microbenchmark support library
Apache License 2.0
8.93k stars 1.61k forks source link

[bazel] Use `includes` instead of `strip_include_prefix` #1803

Closed alexkaratarakis closed 3 months ago

alexkaratarakis commented 3 months ago

When using includes, consumers will apply the headers using -isystem, instead of -I. This will allow diagnostics of consumers to not apply to benchmark.

More info:

https://bazel.build/reference/be/c-cpp#cc_library.includes

https://bazel.build/reference/be/c-cpp#cc_library.strip_include_prefix

gtest uses includes as well: https://github.com/google/googletest/blob/1d17ea141d2c11b8917d2c7d029f1c4e2b9769b2/BUILD.bazel#L120

dmah42 commented 3 months ago

i had originally decided to not use includes for exactly this reason: that it is not a system include and would normally be included with a manual installation as -I rather than -isystem. however, i recognise the diagnostic issues that can be exposed through this, and that this PR simplifies integration for clients, so i'm happy to accept the change.

thanks so much :)

dmah42 commented 3 months ago

looks like precommit has an opinion about ordering of fields.

alexkaratarakis commented 3 months ago

looks like precommit has an opinion about ordering of fields.

Fixed and verified locally:

$ pre-commit run --all-files
buildifier...............................................................Passed
buildifier-lint..........................................................Passed
mypy.....................................................................Passed
ruff.....................................................................Passed
ruff-format..............................................................Passed
dmah42 commented 3 months ago

unrelated failure. thank you for the fix.