Inclusion request review into conan-center

[danimtb]

Inclusion Request Review

This is a review for google_benchmark/1.4.1@tsoft/stable (tsoft/public-conan) inclusion request to conan-center.
This review follows the inclusion guidelines of third party libraries.

Package

google_benchmark/1.4.1@tsoft/stable

Checks and Comments

Recipe quality

• Git public repository
• Recipe fields
• Linter: There are some linting warnings. Please try to solve them:

WARN: Linter. Line 70: TODO: on Solaris add "kstat"

• Updated: CMake helper already manages fPIC flag automatically. Please remove:

cmake.definitions['CMAKE_POSITION_INDEPENDENT_CODE'] = "ON" if self.options.fPIC else "OFF"

Please indicate license of library explicitly in license attribute.

• Clean: Many comments in the recipe. Are them needed?
• test_package
• Raise errors on invalid configurations
• LICENSE of the recipe
• LICENSE of the library

CI Integration

• Provide at least one CI configuration (header only)
• (If not header only) CI configurations for Linux/MacOSX/Windows

Bintray Package Information

• Description
• Licenses
• Tags
• Maturity
• Website (if any)
• Issue tracker
• Version control

Please complete the Bintray metadata indicated above

Additional comments

Missing packages for apple-clang 9.1, Linux clang 5 and 6 and Linux GCC 8. Please include them in your Travis CI like this:

      - <<: *linux
        env: CONAN_GCC_VERSIONS=8 CONAN_DOCKER_IMAGE=lasote/conangcc8
      - <<: *linux
        env: CONAN_CLANG_VERSIONS=5.0 CONAN_DOCKER_IMAGE=lasote/conanclang50
      - <<: *linux
        env: CONAN_CLANG_VERSIONS=6.0 CONAN_DOCKER_IMAGE=lasote/conanclang60
      - <<: *osx
        osx_image: xcode9.3
        env: CONAN_APPLE_CLANG_VERSIONS=9.1

I think there is a missing optional dependency from gtest: https://github.com/google/benchmark/blob/1f35fa4aa71bffb5e5672f7ca876561d6adef4fd/CMakeLists.txt#L26
Check also the readme: https://github.com/google/benchmark#building

Finally, do we really want to name it "google_benchmark"? Is "benchmark" not right? Are there other many "benchmark" libraries apart from the google's one? I think we should stick to the project name.

Please do not hesitate to ask if any doubt and I will be happy to help and discuss the changes requested. Hope everything makes sense! 😄

[Croydon]

The benchmark people are interested in official Conan support, maybe consider helping the efforts over there? google/benchmark#635

[danimtb]

Hi @Croydon and thanks a lot for pointing to that valuable information.

Currently we have 2 inclusion requests for this library and now the benchmark people interested in Conan. So I guess best option will be to wait for them to include official support.

I will keep this open until we know how the official recipe develops. Thanks a lot guys!

[Croydon]

@danimtb @raulbocanegra I have prepared here an in-source recipe. I would appreciate some feedback 😄

https://github.com/Croydon/benchmark

[danimtb]

@Croydon looking quite good and non intrusive enough to make a decent PR with the Conan stuff! Great job

[raulbocanegra]

Hi @Croydon I had a look at it and it seems ok. I only have one doubt: windows 32 bits builds are not supported, so could it be fine to raise an exception as you do with shared builds?

[Croydon]

From my understanding in google/benchmark#447 32bit Windows builds are supported, you just don't need to set the BENCHMARK_BUILD_32_BITS option to true, if you would, then the -m32 flag would get appended, which MSVC doesn't understand and therefore fails. 32bit Windows builds should be fine.

PR is in progress, btw, for the case you didn't notice google/benchmark#647 😄

[Croydon]

Benchmark is now in the Conan Center Index: https://github.com/conan-io/conan-center-index/