Build tools: build configurations, vcpkg support, vcpkg port file for OpenTelemetry, use submodules for CMake deps #377
Conversation
| if [[ ! -z "$CHANGED" ]]; then | ||
| echo "The following files have changes:" | ||
| echo "$CHANGED" | ||
| git diff |
There was a problem hiding this comment.
It's a pain when formatting tools fail. At least this should show what changed exactly.
There was a problem hiding this comment.
@jsuereth - I decided to add this small patch in another simpler PR, so that it gets in sooner.
There was a problem hiding this comment.
@pyohannes - I updated the cmake formatting tools to latest, which matches latest as of now in Ubuntu 20.04. Test it all works in both 18.04 and 20.04, have not tested in other distros. Fixes #375
Codecov Report
@@ Coverage Diff @@
## master #377 +/- ##
==========================================
+ Coverage 94.74% 94.77% +0.02%
==========================================
Files 175 175
Lines 7591 7591
==========================================
+ Hits 7192 7194 +2
+ Misses 399 397 -2
|
|
Bazel on Mac is broken (unrelated to my commit, Bazel is being flaky, disappeared after rerun): ERROR: /Users/runner/work/opentelemetry-cpp/opentelemetry-cpp/examples/zpages/BUILD:17:10: Target '//examples/zpages:zpages_example' depends on toolchain '@local_config_cc//:cc-compiler-darwin', which cannot be found: error loading package '@local_config_cc//': cannot load '@local_config_cc_toolchains//:osx_archs.bzl': no such file'
ERROR: Analysis of target '//examples/zpages:zpages_example' failed; build aborted: Analysis failed
INFO: Elapsed time: 232.660s
INFO: 0 processes.
FAILED: Build did NOT complete successfully (54 packages loaded, 103 targets configured)
FAILED: Build did NOT complete successfully (54 packages loaded, 103 targets configured)
Error: Process completed with exit code 1. |
|
What's the exact intention of those scripts? Are they meant to replace the setup scripts we currently use in our CI workflow? |
I'm afraid this still doesn't solve the code formatting issues, as for that we're relying on |
These are meant to be used for local dev builds. Some of it could be used by CI. Current CI coverage for Windows / msvc is incomplete. These scripts add the necessary setup for vs2015/vs2017/vs2019. Some of it is relevant w.r.t. how you'd operate with CMake and
There was no intent to cover this part. |
|
There is another practical reason why I'd like to integrate this change first: my other change (STL) may depend on some of it. Integrating this |
|
I can provide a separate write-up on how to use this and Visual Studio IDE to streamline the work with any compiler (vs2015, vs2017, vs2019) and |
Keep third_party/json reserved for CMake submodule build. Remove top-level cmake/TBD as it is not needed and not used.
| REM By default we generate the project for the older Visual Studio 2017 even if we have newer version installed | ||
| cmake ../ -G %CMAKE_GEN% -Ax64 -DBENCHMARK_ENABLE_TESTING=OFF | ||
| set SOLUTION=%ROOT%\third_party\benchmark\build\benchmark.sln | ||
| msbuild %SOLUTION% /maxcpucount:%MAXCPUCOUNT% /p:Configuration=Debug /p:Platform=x64 |
There was a problem hiding this comment.
Use cmake --build . instead of calling msbuild to be consistent?
There was a problem hiding this comment.
We may be able to remove this script, as right now we have a few ways to inject the benchmark via vcpkg.
There was a problem hiding this comment.
I'll keep it as-is for now. It's not used right now by the outside build.cmd script. I'll probably remove this build-benchmark.cmd in a follow-up PR.
tools/install-vs-addons.cmd
Outdated
| REM Install optional components required for ARM build - vs2017-BuildTools | ||
| IF EXIST "%ProgramFiles(x86)%\Microsoft Visual Studio\2017\BuildTools" ( | ||
| "%ProgramFiles(x86)%\Microsoft Visual Studio\Installer\vs_installer.exe" ^ | ||
| -- modify --installPath "%ProgramFiles(x86)%\Microsoft Visual Studio\2017\BuildTools" -q ^ |
There was a problem hiding this comment.
Curious whether the -- before modify is necessary?
There was a problem hiding this comment.
Good question. Let me follow-up on this.
There was a problem hiding this comment.
I think it wasn't necessary. Not sure where it came from. It was benign and I removed the empty param.
|
I'll fix the formatting. It's because I upgraded the formatting tools - it got broken after update 😄 |
Co-authored-by: Johannes Tax <[email protected]>
maxgolov
added
the
pr:please-merge
[CONFIGURATION] File configuration - trace model (open-telemetry#3467)
List of tools
build.cmd- builds OpenTelemetry using selected Visual Studio version (2015, 2017, 2019). Could be done side-by-side on one machine, as their output directories would differ. This allows building and comparing the output across all compilers at once. I also added an option to pass custom build flags for 2017 and 2019 (that would be needed for my other PR). Section related to-DWITH_STL. This has no effect if CMakeLists.txt does not have a corresponding build option.build-benchmark.cmd- builds Google Test & Google Benchmark with vs2019 from pinned submodule commit idbuild-gtest.sh- builds Google Test without samples and tests for POSIX OS (Linux or Mac)build-vs2015.cmdbuild-vs2017.cmdbuild-vs2019.cmd- are shortcuts to setup corresponding build tools first, then build using CMake, targeting selected version. Build scripts use the default toolchain for each version:2015 - vc140 - C++11/142017 - vc141 - C++172019 - vc142 - C++20Assuming that
CMakeLists.txtof the product contains correspondingCMAKE_CXX_STANDARDto decide what language standard to use. Lowest, 11, is applied if the code is compiled with vs2015 (vc140). Highest, 20, is applied if the code is compiled with vs2019 (vc142).Developer experience
When dev has only one compiler installed on their machine, the scripts would sort out what compiler to use automagically.
For example:
tools\build.cmd- builds for Windows using whatever installed compiler. Preference is given to latest.tools/build.sh- builds for either Linux or Mac using whatever installed defaultCCcompiler.Additional setup scripts
setup-buildtools-mac.sh- standalone script to setup CMake + dependencies usingbrewsetup-buildtools.sh- standalone script to setup Debian, Ubuntu or RedHat packages needed to build with CMakesetup-buildtools.cmd- standalone script to setup all deps + vcpkg using chocolatey package manager (available by default on GitHub Actions runner). This script also integrates all deps into current Visual Studio installation.Submodules and Reasons why adding submodules is beneficial for CMake build system
Unfortunately package managers across different distros do not typically allow to easily pin to given dependency version. Recent example: our CI code formatting loop behaves differently on Ubuntu 18.x vs 20.xx; same comment generally applies to
vcpkg- it is a bit harder to pin to given dependency version (need to locally cache given version of CONTROL file).When we add submodule - we can control what upstream git repo we track, what branch, and can pin submodule to specific Commit ID. That way we shield ourselves from spontaneous changes in any of our dependencies. This would be the most predictable setup (somewhat more predictable than
vcpkg) for our official releases.Whereas when we do CI - it is usually easier to test with dependency version coming either from OS package manager:
rpmyumaptdebvcpkgbrewThus, I'm including both - examples how to install 'quickly' using OS-provided packager manager; versus how to build needed test deps from source, originating from git submodule.
Note on GSL: Guidelines Support Library
The Guidelines Support Library (GSL) contains functions and types that are suggested for use by the C++ Core Guidelines maintained by the Standard C++ Foundation... The entire implementation is provided inline in the headers under the gsl directory. The implementation generally assumes a platform that implements C++14 support.
This could be used as optional dependency for header-only build of SDK using
C++14compatible compiler, such as Visual Studio 2015. This library is MIT-licensed, not required by default by OpenTelemetry C++ SDK, and not to be used by prebuilt binary distribution of OpenTelemetry C++ SDK. Main logical reasoning here - GSL could be used as the lightest header only way to compile the code with STANDARD Library, without using non-standard C++ Library provided by OpenTelemetry C++ SDK. Thus, resulting min-size build for header-only SDK; OR for SDK embedded / statically linked in products running on Windows OS.Other OS, esp. any system with C++17 compiler - would not benefit from MS-GSL library.