Add support for building with Bazel.#533
Conversation
c056c3e to
81a9620
Compare
Limitations compared to existing CMake rules: * Defaults to using C++11 `<regex>`, with an override via Bazel flag `--define` of `google_benchmark.have_regex`. The TravisCI config sets the regex implementation to `posix` because it uses ancient compilers. * Debug vs Opt mode can't be set per test. TravisCI runs all the tests in debug mode to satisfy `diagnostics_test`, which depends on `CHECK` being live.
81a9620 to
ce54db3
Compare
|
✅ Build benchmark 1075 completed (commit 37eee8aac9 by @jmillikin-stripe) |
|
✅ Build benchmark 1079 completed (commit 111cf3821f by @jmillikin-stripe) |
|
Dominic, will you have time to review this PR, or should I find someone else? |
| @@ -0,0 +1,7 @@ | |||
| load("@bazel_tools//tools/build_defs/repo:git.bzl", "git_repository") | |||
|
|
|||
| git_repository( | |||
There was a problem hiding this comment.
as per bazel, it may be worth changing this to http_archive: https://docs.bazel.build/versions/master/be/workspace.html
There was a problem hiding this comment.
I'd like to leave this as git_repository, because the auto-generated tarballs created by GitHub don't have stable checksums. See tensorflow/tensorflow#12979 and bazelbuild/bazel#3737 for some examples of GitHub breaking builds by tweaking their tarball builder.
http_repository does work well for GitHub "release" tarballs uploaded by maintainers, such as the Protobuf releases. Googletest doesn't have any of those, though.
|
Other than the minor thing above (which could be a followup) I'm comfortable with this PR. And thank you for including the tests and travis changes. It seems that maintenance is relatively sane too given the use of globs. Let me know if you want to replace the git_repository or not and then I'll merge. |
This is recommended by the Bazel style guide to avoid each dependent workspace defining its own name for the dependency.
|
❌ Build benchmark 1102 failed (commit 950de57c23 by @jmillikin-stripe) |
|
That AppVeyor failure looks unrelated to this PR, since it occurred in the cmake build. |
|
I agree, it looks like the appveyor failure is due to flakiness in the regex where the tests run slower than expected. |
|
Looks good. Thanks for doing this. |
|
thank you! |
* Add myself to CONTRIBUTORS under the corp CLA for Stripe, Inc. * Add support for building with Bazel. Limitations compared to existing CMake rules: * Defaults to using C++11 `<regex>`, with an override via Bazel flag `--define` of `google_benchmark.have_regex`. The TravisCI config sets the regex implementation to `posix` because it uses ancient compilers. * Debug vs Opt mode can't be set per test. TravisCI runs all the tests in debug mode to satisfy `diagnostics_test`, which depends on `CHECK` being live. * Set Bazel workspace name so other repos can refer to it by stable name. This is recommended by the Bazel style guide to avoid each dependent workspace defining its own name for the dependency.
* Add myself to CONTRIBUTORS under the corp CLA for Stripe, Inc. * Add support for building with Bazel. Limitations compared to existing CMake rules: * Defaults to using C++11 `<regex>`, with an override via Bazel flag `--define` of `google_benchmark.have_regex`. The TravisCI config sets the regex implementation to `posix` because it uses ancient compilers. * Debug vs Opt mode can't be set per test. TravisCI runs all the tests in debug mode to satisfy `diagnostics_test`, which depends on `CHECK` being live. * Set Bazel workspace name so other repos can refer to it by stable name. This is recommended by the Bazel style guide to avoid each dependent workspace defining its own name for the dependency.
Limitations compared to existing CMake rules:
<regex>, with an override via Bazel flag--defineofgoogle_benchmark.have_regex. The TravisCI config sets the regex implementation toposixbecause it uses ancient compilers.diagnostics_test, which depends onCHECKbeing live.If accepted, this would obsolete PRs #496 and #501.
I plan to use this for Envoy to replace build_recipes/benchmark.sh.