Skip to content
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

Windows compile with CMake fails - Can't find googletest or nlohmann_json #843

Closed
BuildMan-Alpha opened this issue Jun 10, 2021 · 17 comments · Fixed by #865 or #870
Closed

Windows compile with CMake fails - Can't find googletest or nlohmann_json #843

BuildMan-Alpha opened this issue Jun 10, 2021 · 17 comments · Fixed by #865 or #870
Assignees
Labels
bug Something isn't working

Comments

@BuildMan-Alpha
Copy link

Describe your environment
Windows VS 2019 (16.10.0)

Steps to reproduce
Clone repository with --recursive.
Run CMake with arguments -G "Visual Studio 16 2019" -A "x64" "-DBUILD_TESTING=OFF" "c:\dev\AA3rdPartyExt\opentelemetry-cpp" "-DCMAKE_TOOLCHAIN_FILE=C:\dev\vcpkg\scripts\buildsystems\vcpkg.cmake" -DCMAKE_POSITION_INDEPENDENT_CODE=ON -DWITH_PROMETHEUS=ON

What is the expected behavior?
Clean compile

What is the actual behavior?
Build-CMake-Project-Windows : Error Invoking cmake to create the project files failed - level 1
Building with nostd types...
CMake Error at C:/dev/vcpkg/scripts/buildsystems/vcpkg.cmake:263 (_find_package):
By not providing "Findnlohmann_json.cmake" in CMAKE_MODULE_PATH this
project has asked CMake to find a package configuration file provided by
"nlohmann_json", but CMake did not find one.
Could not find a package configuration file provided by "nlohmann_json"
with any of the following names:
nlohmann_jsonConfig.cmake
nlohmann_json-config.cmake
Add the installation prefix of "nlohmann_json" to CMAKE_PREFIX_PATH or set
"nlohmann_json_DIR" to a directory containing one of the above files. If
"nlohmann_json" provides a separate development package or SDK, be sure it
has been installed.

Additional context
I was able to work around this by cloning and building both googletest and nlohmann_json and setting the CMake CMAKE_PREFIX_PATH environment variable to the target path of the respective builds.

DCMAKE_PREFIX_PATH=c:\dev\AA3rdPartyExt\Build\nlohmann_json\VS16\Win64;c:\dev\AA3rdPartyExt\Build\googletest\VS16\Win64"

Note: It appears that the submodules were downloaded as per the log output below:
Processing project Open Telemetry from repository https://github.com/open-telemetry/opentelemetry-cpp in local folder c:\dev\AA3rdPartyExt\opentelemetry-cpp.
Cloning files from GitHub repository https://github.com/open-telemetry/opentelemetry-cpp into c:\dev\AA3rdPartyExt\opentelemetry-cpp
Submodule path 'third_party/benchmark': checked out 'c05843a9f622db08ad59804c190f98879b76beba'
Submodule path 'third_party/googletest': checked out 'a6dfd3aca7f2f91f95fc7ab650c95a48420d513d'
Submodule path 'third_party/ms-gsl': checked out '6f4529395c5b7c2d661812257cd6780c67e54afa'
Submodule path 'third_party/nlohmann-json': checked out 'db78ac1d7716f56fc9f1b030b715f872f93964e4'
Submodule path 'third_party/opentelemetry-proto': checked out '795cc815ce9bb438b46a4a7f6ecb465b52d50935'
Submodule path 'third_party/prometheus-cpp': checked out 'bb017ec15a824d3301845a1274b4b46a01d6d871'
Submodule path 'third_party/prometheus-cpp/3rdparty/civetweb': checked out '8e243456965c9be5212cb96519da69cd54550e3d'
Submodule path 'third_party/prometheus-cpp/3rdparty/googletest': checked out '703bd9caab50b139428cea1aaff9974ebee5742e'
Submodule path 'tools/vcpkg': checked out 'e9f8cc67a5e5541973e53ac03f88adb45cc1b21b'

@BuildMan-Alpha BuildMan-Alpha added the bug Something isn't working label Jun 10, 2021
@maxgolov
Copy link
Contributor

@BuildMan-Alpha - could you please try this process:

https://github.com/open-telemetry/opentelemetry-cpp/blob/main/docs/building-with-vs2019.md

Adjust the build files to your liking. I'm using this process daily. It works well.

Let me know your feedback.

@maxgolov
Copy link
Contributor

This PR contains a few additional improvements to cover most compilers on Windows (vs2015-vs2019, recent versions of clang) :

#839

@BuildMan-Alpha
Copy link
Author

BuildMan-Alpha commented Jun 11, 2021 via email

@maxgolov
Copy link
Contributor

maxgolov commented Jun 11, 2021

Ninja Generator / build is x5 times faster. And produces the same output. The only scenario where you'd benefit from MSBuild -generated files, is if you want to debug the SDK in older Visual Studio 2017. I hope to get #839 merged, that cleans-up the overall build experience for all compilers - [vs2015|vs2017|vs2019|clang-*]. I will also validate the scripts for Win32 once we merge the fix for the other issue you reported. It should work well. Our main focus has been on x64 so far.

@BuildMan-Alpha
Copy link
Author

BuildMan-Alpha commented Jun 11, 2021 via email

@maxgolov
Copy link
Contributor

PR 839 didn’t seem to affect the Win32 build (assuming the error in circular_buffer.h is yet to be addressed).

Win32 error is being fixed in #847

@maxgolov
Copy link
Contributor

@BuildMan-Alpha

Hi Kurt,

Win32 fix is now merged in the main. Please pull it and try building for Win32.

Re. CMAKE_PREFIX_PATH - this is interesting. We employ two (orthogonal) approaches:

  • nlohmann json from vcpkg installation: scripts that I shared above would use a local snapshot of vcpkg package management. And inside that one we install vcpkg install nlohmann-json:%ARCH%-windows , which is x64 by default and may optionally be set to x86. CMake should be able to find it. I do not see the issue with Could not find a package configuration file provided by "nlohmann_json". As long as I run the installation before the build. I think when you run these scripts, your build is also successful?... albeit with ninja, but ninja is a technicality - it could have been MSBuild.

  • nlohmann json from submodule: we also include a local snapshot of nlohmann json library, which you get in your workspace at third_party/nlohmann-json here , assuming you recursively cloned opentelemetry-cpp repository. Which appears like you did. There's one spot where we fallback to that submodule, assuming that your CMake installation (or vcpkg installation) did not provide the package. For example, piece of logic here - just uses the nlohmann json from submodule:

    find_package(nlohmann_json QUIET)

Perhaps we should improve our top-level CMakeLists.txt to fallback to local submodules, if customer's system does not have the package already installed.

Ultimately there is no right or wrong approach. What you are doing (providing your own dependency to CMake) - is also perfectly valid approach. Is this issue blocking you, would you consider adopting vcpkg as part of your build process on Windows (that'll give you some stable versions of GTest and nlohmann json)? Appears like you tested that this flow works okay. Or do you require specific versions of the packages? In that case what you are doing, providing your own dependencies - is the best thing to do.

@BuildMan-Alpha
Copy link
Author

BuildMan-Alpha commented Jun 15, 2021 via email

@BuildMan-Alpha
Copy link
Author

BuildMan-Alpha commented Jun 15, 2021 via email

@maxgolov maxgolov self-assigned this Jun 15, 2021
@maxgolov
Copy link
Contributor

maxgolov commented Jun 15, 2021

I'll try to improve the handling of this, i.e. if outside CMake is not set up explicitly with the knowledge of GTest and nlohmann_json, then we should use the versions provided by our git submodules. Assigning to myself. But it'd be relatively low priority, since we already document how to build with vcpkg -provided dependencies.

@maxgolov
Copy link
Contributor

maxgolov commented Jun 17, 2021

It's a bit more tricky for GTest (which may not be required for the majority of build pipelines anyways, since it's only for the tests). And more straightforward for nlohmann/json.hpp . I sent out the fix to auto-discover the submodule-provided version of nlohmann/json.hpp in PR #865 .. at least partially alleviating your original concern.

@BuildMan-Alpha
Copy link
Author

BuildMan-Alpha commented Jun 17, 2021 via email

@maxgolov
Copy link
Contributor

@BuildMan-Alpha - indeed, I agree we can improve the documentation piece for the GTest handling. Presently we manually build that in our integration loops. And the scripts I shared with you - these install vcpkg install gtest, essentially doing what you did. vcpkg uses common location for all CMake packages, whereas you are manually specifying the path to your own prebuilt GTest ( CMAKE_PREFIX_PATH pointing to your own build of it ). I merged the fix for nlohmann::json pakage. I'll consider a follow-up improvement for the GTest.

Perhaps we can also document the installation of GTest in either of these documents:
https://github.com/open-telemetry/opentelemetry-cpp/blob/main/docs/building-with-vcpkg.md
https://github.com/open-telemetry/opentelemetry-cpp/blob/main/docs/building-with-vs2019.md
?

For Unix OS - there's that write-up that explains how to install the GTest manually, for it to get auto-discovered by CMake:
https://github.com/open-telemetry/opentelemetry-cpp/blob/main/docs/google-test.md#cmake

@BuildMan-Alpha
Copy link
Author

BuildMan-Alpha commented Jun 18, 2021 via email

@maxgolov maxgolov reopened this Jun 18, 2021
@maxgolov
Copy link
Contributor

maxgolov commented Jun 18, 2021

@BuildMan-Alpha - apologies, I realized why it's breaking on Windows. As a workaround, you have two options:

  • use vcpkg to manage your dependencies - vcpkg install nlohmann-json:x86-windows , then you may point your build to vcpkg via set VCPKG_ROOT="C:\vcpkg" - whatever directory where you have installed and built vcpkg... Then pass -DCMAKE_TOOLCHAIN_FILE="%VCPKG_ROOT}\scripts\buildsystems\vcpkg.cmake" parameter, to point CMake to vcpkg- installed dependency.

  • or somehow otherwise build and install the JSON that we have available in the submodule.

I agree we need to fix this properly, since we do not have to force users to install their own instance of nlohmann::json. I tested this on Linux, but on Windows there's a special case with ETW exporter that needs fixing. I'll send out the fix for review promptly.

@maxgolov
Copy link
Contributor

maxgolov commented Jun 18, 2021

@BuildMan-Alpha - I realized there is a much nicer and easier workaround available for your build system. You do not need an immediate fix. You can add -DJSON_Install=ON option to your CMake command. That should solve the build issue for x86 / ETW exporter.

Short explanation: we do ship submodule of nlohmann::json library. But we were not installing it / not adding it to CMake cache. That caused an issue with ETW exporter, because it requires the nlohmann::json library to be installed / available in CMake list of packages. Other components, e.g. Linux builds - were fine as-is. Because Linux builds do not require the installation of json, only Windows ETW exporter build requires it.

Please try it by running CMake with your old arguments, plus -DJSON_Install=ON. Let me know if it works for you.

Meanwhile I'll submit a change to make this option to be ON by default. So you won't be needing to pass the JSON_Install parameter in the next release anymore. That'll make the overall onboarding experience a bit smoother.

@BuildMan-Alpha
Copy link
Author

BuildMan-Alpha commented Jun 21, 2021 via email

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment