-
Notifications
You must be signed in to change notification settings - Fork 10.5k
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
Update Google Benchmark v1.5.0 to get CMake < 3.6 fix #19535
Conversation
d15a197
to
d9cc1af
Compare
I just rebased atop master as of commit 73a3ff8. |
This PR is including many other PR's unexpectedly? |
d9cc1af
to
199d52e
Compare
@AspirinSJL: not sure what happened. I just rebased again atop a914d1c and the GitHub display looks cleaner now. |
Google Benchmark v1.4.1 uses a CMake feature that is only in version >= 3.6. This was an inadvertent change in Google Benchmark (too high of a version) that has been [fixed upstream][1] in v1.5.0. Google Benchmark v1.5.0 now requires CMake >= 3.5.1. [Another PR][2] will bump gRPC's minimum version as well. [1]: google/benchmark@505be96 [2]: grpc#19467
199d52e
to
09c55da
Compare
Rebased atop 073b234. |
@jtattermusch Could you please take a look at this PR? It's >1 month old. Thanks! |
Sorry for the delay. I think this PR is slightly problematic because cmake 3.5.1 is not available on all of our testing images (hence the failures) and the images will need to be updated first which requires some work (which I haven't gotten to yet). Updating a submodule also requires updating a few more extra file (we have sanity checks in place), but updating those is relatively easy. |
I made the other changes required for bumping the version. Let's see what the test say (I expect we'll still need to upgrade cmake in some of our testing images) |
Thanks for fixing the other files. We picked CMake 3.5.1 as that's what's in Debian stable and Ubuntu |
adhoc artifact / packages build: https://sponge.corp.google.com/invocation?id=ff0f1920-5ec4-435d-a4f7-1c10bfb7848c&searchFor= |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM, but I will still run some adhoc tests.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Retroactive LGTM
Google Benchmark v1.4.1 uses a CMake feature that is only in version >=
3.6. This was an inadvertent change in Google Benchmark (too high of a
version) that has been fixed upstream in v1.5.0.
Google Benchmark v1.5.0 now requires CMake >= 3.5.1. Another PR
will bump gRPC's minimum version as well.
This PR supersedes #19466.