|
| 1 | +# The build system |
| 2 | + |
| 3 | +We are currently migrating from `autotools` to `CMake` as a build-system. This document |
| 4 | +currently describes how we intend to perform this migration, and will be updated after |
| 5 | +the migration to explain how the new `CMake` configuration works. |
| 6 | + |
| 7 | +## Stages during the build |
| 8 | + |
| 9 | +1. The `netdata-installer.sh`, take in arguments and environment settings to control the |
| 10 | + build. |
| 11 | +2. The configure step: `autoreconf -ivf ; ./configure` passing arguments into the configure |
| 12 | + script. This becomes `generation-time` in CMake. This includes package / system detection |
| 13 | + and configuration resulting in the `config.h` in the source root. |
| 14 | +3. The build step: recurse through the generated Makefiles and build the executable. |
| 15 | +4. The first install step: calls `make install` to handle all the install steps put into |
| 16 | + the Makefiles by the configure step (puts binaries / libraries / config into target |
| 17 | + tree structure). |
| 18 | +5. The second install step: the rest of the installer after the make install handles |
| 19 | + system-level configuration (privilege setting, user / groups, fetch/build/install `go.d` |
| 20 | + plugins, telemetry, installing service for startup, uninstaller, auto-updates. |
| 21 | + |
| 22 | +The ideal migration result is to replace all of this with the following steps: |
| 23 | +``` |
| 24 | +mkdir build ; cd build ; cmake .. -D... ; cmake --build . --target install |
| 25 | +``` |
| 26 | + |
| 27 | +The `-D...` indicates where the command-line arguments for configuration are passed into |
| 28 | +`CMake`. |
| 29 | + |
| 30 | +## CMake generation time |
| 31 | + |
| 32 | +At generation time we need to solve the following issues: |
| 33 | + |
| 34 | +### Feature flags |
| 35 | + |
| 36 | +Every command-line switch on the installer and the configure script needs to becomes an |
| 37 | +argument to the CMake generation, we can do this with variables in the CMake cache: |
| 38 | + |
| 39 | +CMakeLists.txt: |
| 40 | +``` |
| 41 | +option(ENABLE_DBENGINE "Enable the dbengine storage" ON) |
| 42 | +... |
| 43 | +if(${ENABLE_DBENGINE}) |
| 44 | +... |
| 45 | +endif() |
| 46 | +``` |
| 47 | + |
| 48 | +Command-line interface |
| 49 | +``` |
| 50 | +cmake -DENABLE_DBENGINE |
| 51 | +``` |
| 52 | + |
| 53 | +### Dependency detection |
| 54 | + |
| 55 | +We have a mixture of soft- and hard-depedencies on libraries. For most of these we expect |
| 56 | +`pkg-config` information, for some we manually probe for libraries and include files. We |
| 57 | +should treat all of the external dependencies consistently: |
| 58 | + |
| 59 | +1. Default to autodetect using `pkg-config` (e.g. the standard `jemalloc` drops a `.pc` |
| 60 | + into the system but we do not check for it. |
| 61 | +2. If no `.pc` is found perform a manual search for libraries under known names, and |
| 62 | + check for accessible symbols inside them. |
| 63 | +3. Check that include paths work. |
| 64 | +4. Allow a command-line override (e.g. `-DWITH_JEMALLOC=/...`). |
| 65 | +5. If none of the above work then fail the install if the dependency is hard, otherwise |
| 66 | + indicate it is not present in the `config.h`. |
| 67 | + |
| 68 | +Before doing any dependency detection we need to determine which search paths are |
| 69 | +really in use for the current compiler, after the `project` declaration we can use: |
| 70 | +``` |
| 71 | +execute_process(COMMAND ${CMAKE_C_COMPILER} "--print-search-dirs" |
| 72 | + COMMAND grep "^libraries:" |
| 73 | + COMMAND sed "s/^libraries: =//" |
| 74 | + COMMAND tr ":" " " |
| 75 | + COMMAND tr -d "\n" |
| 76 | + OUTPUT_VARIABLE CC_SEARCH_DIRS |
| 77 | + RESULTS_VARIABLE CC_SEARCH_RES) |
| 78 | +string(REGEX MATCH "^[0-9]+" CC_SEARCH_RES ${CC_SEARCH_RES}) |
| 79 | +#string(STRIP "${CC_SEARCH_RES}" CC_SEARCH_RES) |
| 80 | +if(0 LESS ${CC_SEARCH_RES}) |
| 81 | + message(STATUS "Warning - cannot determine standard compiler library paths") |
| 82 | + # Note: we will probably need a different method for Windows... |
| 83 | +endif() |
| 84 | +
|
| 85 | +``` |
| 86 | + |
| 87 | +The output format for this switch works on both `Clang` and `gcc`, it also includes |
| 88 | +the include search path, which can be extracted in a similar way. Standard advice here |
| 89 | +is to list the `ldconfig` cache or use the `-V` flag to check, but this does not work |
| 90 | +consistently across platforms - in particular `gcc` will reconfigure `ld` when it is |
| 91 | +called to gcc's internal view of search paths. During experiments each of these |
| 92 | +alternative missed / added unused paths. Dumping the compiler's own estimate of the |
| 93 | +search paths seems to work consistently across clang/gcc/linux/freebsd configurations. |
| 94 | + |
| 95 | +The default behaviour in CMake is to search across predefined paths (e.g. `CMAKE_LIBRARY_PATH`) |
| 96 | +that are based on heuristics about the current platform. Most projects using CMake seem |
| 97 | +to overwrite this with their own estimates. |
| 98 | + |
| 99 | +We can use the extracted paths as a base, add our own heuristics based on OS and then |
| 100 | +`set(CMAKE_LIBRARY_PATH ${OUR_OWN_LIB_SEARCH})` to get the best results. Roughly we do |
| 101 | +the following for each external dependency: |
| 102 | +``` |
| 103 | +set(WITH_JSONC "Detect" CACHE STRING "Manually set the path to a json-c installation") |
| 104 | +... |
| 105 | +if(${WITH_JSONC} STREQUAL "Detect") |
| 106 | + pkg_check_modules(JSONC json-c) # Don't set the REQUIRED flag |
| 107 | + if(JSONC_FOUND) |
| 108 | + message(STATUS "libjsonc found through .pc -> ${JSONC_CFLAGS_OTHER} ${JSONC_LIBRARIES}") |
| 109 | + # ... setup using JSONC_CFLAGS_OTHER JSONC_LIBRARIES and JSONC_INCLUDE_DIRS |
| 110 | + else() |
| 111 | + find_library(LIB_JSONC |
| 112 | + NAMES json-c libjson-c |
| 113 | + PATHS ${CMAKE_LIBRARY_PATH}) # Includes our additions by this point |
| 114 | + if(${LIB_JSONC} STREQUAL "LIB_JSONC-NOTFOUND") |
| 115 | + message(STATUS "Library json-c not installed, disabling") |
| 116 | + else() |
| 117 | + check_library_exists(${LIB_JSONC} json_object_get_type "" HAVE_JSONC) |
| 118 | + # ... setup using heuristics for CFLAGS and check include files are available |
| 119 | + endif() |
| 120 | + endif() |
| 121 | +else() |
| 122 | + # ... use explicit path as base to check for library and includes ... |
| 123 | +endif() |
| 124 | +
|
| 125 | +``` |
| 126 | + |
| 127 | +For checking the include path we have two options, if we overwrite the `CMAKE_`... variables |
| 128 | +to change the internal search path we can use: |
| 129 | +``` |
| 130 | +CHECK_INCLUDE_FILE(json/json.h HAVE_JSONC_H) |
| 131 | +``` |
| 132 | +Or we can build a custom search path and then use: |
| 133 | +``` |
| 134 | +find_file(HAVE_JSONC_H json/json.h PATHS ${OUR_INCLUDE_PATHS}) |
| 135 | +``` |
| 136 | + |
| 137 | +Note: we may have cases where there is no `.pc` but we have access to a `.cmake` (e.g. AWS SDK, mongodb,cmocka) - these need to be checked / pulled inside the repo while building a prototype. |
| 138 | + |
| 139 | +### Compiler compatability checks |
| 140 | + |
| 141 | +In CMakeLists.txt: |
| 142 | + |
| 143 | +``` |
| 144 | +CHECK_INCLUDE_FILE(sys/prctl.h HAVE_PRCTL_H) |
| 145 | +configure_file(cmake/config.in config.h) |
| 146 | +``` |
| 147 | + |
| 148 | +In cmake/config.in: |
| 149 | + |
| 150 | +``` |
| 151 | +#cmakedefine HAVE_PRCTL_H 1 |
| 152 | +``` |
| 153 | + |
| 154 | +If we want to check explicitly if something compiles (e.g. the accept4 check, or the |
| 155 | +`strerror_r` typing issue) then we set the `CMAKE_`... paths and then use: |
| 156 | +``` |
| 157 | +check_c_source_compiles( |
| 158 | + " |
| 159 | + #include <string.h> |
| 160 | + int main() { char x = *strerror_r(0, &x, sizeof(x)); return 0; } |
| 161 | + " |
| 162 | + STRERROR_R_CHAR_P) |
| 163 | +
|
| 164 | +``` |
| 165 | +This produces a bool that we can use inside CMake or propagate into the `config.h`. |
| 166 | + |
| 167 | +We can handle the atomic checks with: |
| 168 | +``` |
| 169 | +check_c_source_compiles( |
| 170 | + " |
| 171 | + int main (int argc, char **argv) |
| 172 | + { |
| 173 | + volatile unsigned long ul1 = 1, ul2 = 0, ul3 = 2; |
| 174 | + __atomic_load_n(&ul1, __ATOMIC_SEQ_CST); |
| 175 | + __atomic_compare_exchange(&ul1, &ul2, &ul3, 1, __ATOMIC_SEQ_CST, __ATOMIC_SEQ_CST); |
| 176 | + __atomic_fetch_add(&ul1, 1, __ATOMIC_SEQ_CST); |
| 177 | + __atomic_fetch_sub(&ul3, 1, __ATOMIC_SEQ_CST); |
| 178 | + __atomic_or_fetch(&ul1, ul2, __ATOMIC_SEQ_CST); |
| 179 | + __atomic_and_fetch(&ul1, ul2, __ATOMIC_SEQ_CST); |
| 180 | + volatile unsigned long long ull1 = 1, ull2 = 0, ull3 = 2; |
| 181 | + __atomic_load_n(&ull1, __ATOMIC_SEQ_CST); |
| 182 | + __atomic_compare_exchange(&ull1, &ull2, &ull3, 1, __ATOMIC_SEQ_CST, __ATOMIC_SEQ_CST); |
| 183 | + __atomic_fetch_add(&ull1, 1, __ATOMIC_SEQ_CST); |
| 184 | + __atomic_fetch_sub(&ull3, 1, __ATOMIC_SEQ_CST); |
| 185 | + __atomic_or_fetch(&ull1, ull2, __ATOMIC_SEQ_CST); |
| 186 | + __atomic_and_fetch(&ull1, ull2, __ATOMIC_SEQ_CST); |
| 187 | + return 0; |
| 188 | + } |
| 189 | + " |
| 190 | + HAVE_C__ATOMIC) |
| 191 | +``` |
| 192 | + |
| 193 | +For the specific problem of getting the correct type signature in log.c for the `strerror_r` |
| 194 | +calls we can replicate what we have now, or we can delete this code completely and use a |
| 195 | +better solution that is documented [here](http://www.club.cc.cmu.edu/~cmccabe/blog_strerror.html). |
| 196 | +To replicate what we have now: |
| 197 | +``` |
| 198 | +check_c_source_compiles( |
| 199 | + " |
| 200 | + #include <string.h> |
| 201 | + int main() { char x = *strerror_r(0, &x, sizeof(x)); return 0; } |
| 202 | + " |
| 203 | + STRERROR_R_CHAR_P) |
| 204 | +
|
| 205 | +check_c_source_compiles( |
| 206 | + " |
| 207 | + #include <string.h> |
| 208 | + int main() { int x = strerror_r(0, &x, sizeof(x)); return 0; } |
| 209 | + " |
| 210 | + STRERROR_R_INT) |
| 211 | +
|
| 212 | +if("${STRERROR_R_CHAR_P}" OR "${STRERROR_R_INT}") |
| 213 | + set(HAVE_DECL_STRERROR_R 1) |
| 214 | +endif() |
| 215 | +message(STATUS "Result was ${HAVE_DECL_STRERROR_R}") |
| 216 | +
|
| 217 | +``` |
| 218 | + |
| 219 | +Note: I did not find an explicit way to select compiler when both `clang` and `gcc` are |
| 220 | +present. We might have an implicit way (like redirecting `cc`) but we should put one in. |
| 221 | + |
| 222 | + |
| 223 | + |
| 224 | +### Debugging problems in test compilations |
| 225 | + |
| 226 | +Test compilations attempt to feed a test-input into the targetted compiler and result |
| 227 | +in a yes/no decision, this is similar to `AC_LANG_SOURCE(.... if test $ac_...` in .`m4`. |
| 228 | +We have two techniques to use in CMake: |
| 229 | +``` |
| 230 | +cmake_minimum_required(VERSION 3.1.0) |
| 231 | +include(CheckCCompilerFlag) |
| 232 | +project(empty C) |
| 233 | +
|
| 234 | +check_c_source_compiles( |
| 235 | + " |
| 236 | + #include <string.h> |
| 237 | + int main() { char x = *strerror_r(0, &x, sizeof(x)); return 0; } |
| 238 | + " |
| 239 | + STRERROR_R_CHAR_P) |
| 240 | +
|
| 241 | +try_compile(HAVE_JEMALLOC ${CMAKE_CURRENT_BINARY_DIR} |
| 242 | + ${CMAKE_CURRENT_SOURCE_DIR}/quickdemo.c |
| 243 | + LINK_LIBRARIES jemalloc) |
| 244 | +``` |
| 245 | + |
| 246 | +The `check_c_source_compiles` is light-weight: |
| 247 | + |
| 248 | +* Inline source for the test, easy to follow. |
| 249 | +* Build errors are reported in `CMakeFiles/CMakeErrors.log` |
| 250 | + |
| 251 | +But we cannot alter the include-paths / library-paths / compiler-flags specifically for |
| 252 | +the test without overwriting the current CMake settings. The alternative approach is |
| 253 | +slightly more heavy-weight: |
| 254 | + |
| 255 | +* Can't inline source for `try_compile` - it requires a `.c` file in the tree. |
| 256 | +* Build errors are not shown, the recovery process for them is somewhat difficult. |
| 257 | + |
| 258 | +``` |
| 259 | +rm -rf * && cmake .. --debug-trycompile |
| 260 | +grep jemal CMakeFiles/CMakeTmp/CMakeFiles/*dir/* |
| 261 | +cd CMakeFiles/CMakeTmp/CMakeFiles/cmTC_d6f0e.dir # for example |
| 262 | +cmake --build ../.. |
| 263 | +``` |
| 264 | + |
| 265 | +This implies that we can do this to diagnose problems / develop test-programs, but we |
| 266 | +have to make them *bullet-proof* as we cannot expose this to end-users. This means that |
| 267 | +the results of the compilation must be *crisp* - exactly yes/no if the feature we are |
| 268 | +testing is supported. |
| 269 | + |
| 270 | +### System configuration checks |
| 271 | + |
| 272 | +For any system configuration checks that fall outside of the above scope (includes, libraries, |
| 273 | +packages, test-compilation checks) we have a fall-back that we can use to glue any holes |
| 274 | +that we need, e.g. to pull out the packaging strings, inside the `CMakeLists.h`: |
| 275 | +``` |
| 276 | +execute_process(COMMAND cat ${CMAKE_CURRENT_SOURCE_DIR}/packaging/version |
| 277 | + COMMAND tr -d '\n' |
| 278 | + OUTPUT_VARIABLE VERSION_FROM_FILE) |
| 279 | +message(STATUS "Packaging version ${VERSION_FROM_FILE}") |
| 280 | +``` |
| 281 | +and this in the `config.h.in`: |
| 282 | +``` |
| 283 | +#define VERSION_FROM_FILE "@VERSION_FROM_FILE@" |
| 284 | +``` |
| 285 | + |
| 286 | +## CMake build time |
| 287 | + |
| 288 | +We have a working definition of the targets that is in use with CLion and works on modern |
| 289 | +CMake (3.15). It breaks on older CMake version (e.g. 3.7) with an error message (issue#7091). |
| 290 | +No PoC yet to fix this, but it looks like changing the target properties should do it (in the |
| 291 | +worst case we can drop the separate object completely and merge the sources directly into |
| 292 | +the final target). |
| 293 | + |
| 294 | +Steps needed for building a prototype: |
| 295 | + |
| 296 | +1. Pick a reasonable configuration. |
| 297 | +2. Use the PoC techniques above to do a full generation of `CMAKE_` variables in the cache |
| 298 | + according to the feature options and dependencies. |
| 299 | +3. Push these into the project variables. |
| 300 | +4. Work on it until the build succeeds in at least one known configuration. |
| 301 | +5. Smoke-test that the output is valid (i.e. the executable loads and runs, and we can |
| 302 | + access the dashboard). |
| 303 | +6. Do a full comparison of the `config.h` generated by autotools against the CMake version |
| 304 | + and document / fix any deviations. |
| 305 | + |
| 306 | +## CMake install target |
| 307 | + |
| 308 | +I've only looked at this superficially as we do not have a prototype yet, but each of the |
| 309 | +first-stage install steps (in `make install`) and the second-stage (in `netdata-installer.sh`) |
| 310 | +look feasible. |
| 311 | + |
| 312 | +## General issues |
| 313 | + |
| 314 | +* We need to choose a minimum CMake version that is an available package across all of our |
| 315 | + supported environments. There is currently a build issue #7091 that documents a problem |
| 316 | + in the compilation phase (we cannot link in libnetdata as an object on old CMake versions |
| 317 | + and need to find a different way to express this). |
| 318 | + |
| 319 | +* The default variable-expansion / comparisons in CMake are awkward, we need this to make it |
| 320 | + sane: |
| 321 | + ``` |
| 322 | + cmake_policy(SET CMP0054 "NEW") |
| 323 | + ``` |
| 324 | +* Default paths for libs / includes are not comprehensive on most environments, we still need |
| 325 | + some heuristics for common locations, e.g. `/usr/local` on FreeBSD. |
| 326 | +
|
| 327 | +# Recommendations |
| 328 | +
|
| 329 | +We should follow these steps: |
| 330 | +
|
| 331 | +1. Build a prototype. |
| 332 | +2. Build a test-environment to check the prototype against environments / configurations that |
| 333 | + the team uses. |
| 334 | +3. Perform an "internal" release - merge the new CMake into master, but not announce it or |
| 335 | + offer to support it. |
| 336 | +4. Check it works for the team internally. |
| 337 | +5. Do a soft-release: offer it externally as a replacement option for autotools. |
| 338 | +6. Gather feedback and usage reports on a wider range of configurations. |
| 339 | +7. Do a hard-release: switch over the preferred build-system in the installation instructions. |
| 340 | +8. Gather feedback and usage reports on a wider range of configurations (again). |
| 341 | +9. Deprecate / remove the autotools build-system completely (so that we can support a single |
| 342 | + build-system). |
| 343 | +
|
| 344 | +Some smaller miscellaeneous suggestions: |
| 345 | +
|
| 346 | +1. Remove the `_Generic` / `strerror_r` config to make the system simpler (use the technique |
| 347 | + on the blog post to make the standard version re-enterant so that it is thread-safe). |
| 348 | +2. Pull in jemalloc by source into the repo if it is our preferred malloc implementation. |
| 349 | +
|
| 350 | +# Background |
| 351 | +
|
| 352 | +* [Stack overflow starting point](https://stackoverflow.com/questions/7132862/how-do-i-convert-an-autotools-project-to-a-cmake-project#7680240) |
| 353 | +* [CMake wiki including previous autotools conversions](https://gitlab.kitware.com/cmake/community/wikis/Home) |
| 354 | +* [Commands section in old CMake docs](https://cmake.org/cmake/help/v2.8.8/cmake.html#section_Commands) |
| 355 | +* [try_compile in newer CMake docs](https://cmake.org/cmake/help/v3.7/command/try_compile.html) |
| 356 | +* [configure_file in newer CMake docs](https://cmake.org/cmake/help/v3.7/command/configure_file.html?highlight=configure_file) |
| 357 | +* [header checks in CMake](https://stackoverflow.com/questions/647892/how-to-check-header-files-and-library-functions-in-cmake-like-it-is-done-in-auto) |
| 358 | +* [how to write platform checks](https://gitlab.kitware.com/cmake/community/wikis/doc/tutorials/How-To-Write-Platform-Checks) |
| 359 | +
|
0 commit comments