Re: Review Request 61753: Refactored CMake build system.

[Joseph Wu <jo...@mesosphere.io>]

-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/61753/#review184465
-----------------------------------------------------------


Ship it!




Ship It!

- Joseph Wu


On Aug. 18, 2017, 2:10 p.m., Andrew Schwartzmeyer wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/61753/
> -----------------------------------------------------------
> 
> (Updated Aug. 18, 2017, 2:10 p.m.)
> 
> 
> Review request for mesos and Joseph Wu.
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> This commit deletes extraneous CMake code. With a proper CMake
> dependency graph, CMake automatically calculates the correct compiler
> and linker options based on the traversal of the graph, and the
> properties of each target. With the correct importing of the 3rdparty
> dependencies, this allows us to remove all uses of deprecated CMake
> commands such as `include_directories`, `link_directories`, and
> `link_libraries`. These commands rely on setting directory-wide
> side-effects, unlike their explicit `target_include_directories`
> counterparts, which instead add the information to the dependency graph.
> By utilizing CMake's dependency graph, most uses of `add_dependencies`
> were removed, as an implicit dependency is created when
> `target_link_libraries` is used. Note that there are explicit cases of
> manually dependencies, such as module libraries which are not linked and
> therefore are not automatically added to the graph.
> 
> Furthermore, superfluous variables were removed. Due to how the original
> system was written (based on copying a working template), many variables
> were introduced that contained single items, such that the declaration
> and interpolation of the variable became far more verbose than simply
> using the contents directly. Variables holding target names were also
> removed, as they were redundant. See the changes to
> `src/master/CMakeLists.txt` for an example of an executable's build
> going from twelve logical lines of code to two.
> 
> "Configuration" files such as `AgentConfigure.cmake`, were deleted in
> their entirety, because the information they provided was redundant when
> using the CMake dependency graph. The only CMake configuration files
> remaining are for actual project configuration.
> 
> 
> Diffs
> -----
> 
>   CMakeLists.txt 870bd9ba2d5edfe31092d602135edd28dc4982e0 
>   cmake/CompilationConfigure.cmake 2109697e02e58c673948f2ae67ef8255d16aa736 
>   cmake/MesosConfigure.cmake 5ecad2c0fbd245dd14e9c09869dd6da2ca4e602a 
>   src/CMakeLists.txt 0816c6ebaa9caaa5b6a606e601427ed17112a300 
>   src/checks/CMakeLists.txt 4854b2765202e10f5b791f08ed1416f7740e2aad 
>   src/cli/CMakeLists.txt 056d1d734e4424dac3eb29c323961d3a2e87a8dc 
>   src/docker/CMakeLists.txt 2d7c30db5854853625a99227568a27dde281ec2e 
>   src/examples/CMakeLists.txt e8b7ea4bd91c20536ce7bd8d0ffdfd4e549b056f 
>   src/examples/cmake/ExamplesConfigure.cmake d85da4e2e236df9408c86405caeca4d8eae3a35d 
>   src/launcher/CMakeLists.txt 97edc4a28a995da8a5e963aaa62c4c955cc59719 
>   src/local/CMakeLists.txt 7c9a718d17772be1c08e132136739cfd9449470d 
>   src/log/CMakeLists.txt 2fad6cca467dbde9989d26424a2a6c608cd251e8 
>   src/master/CMakeLists.txt 0062fb3e9d67fdfcda1057112d4ba34abbd4be5f 
>   src/master/cmake/MasterConfigure.cmake 173dc36b5a1b7ff23cd0542ea4313ae6e5e0ae3f 
>   src/slave/CMakeLists.txt 9dbadca53b9ac34db08cfb9c9e9fea741c01114b 
>   src/slave/cmake/AgentConfigure.cmake af2f74f82b68920427f36c0c863b97c0191b8f06 
>   src/slave/container_loggers/CMakeLists.txt c0774ce3110fd8d2aac6dc00d2b800e336214f96 
>   src/slave/containerizer/mesos/CMakeLists.txt fa2043efadd1c863c5ad54fdc518397c97488dcb 
>   src/slave/qos_controllers/CMakeLists.txt 65ab338a71276a77e43af962fbc9b76e050efca6 
>   src/slave/resource_estimators/CMakeLists.txt 1d28bd457c7e567219d892f0940903b12c5a877b 
>   src/tests/CMakeLists.txt 6c05ea93c3288fb9608c191234f95e270390d89e 
>   src/tests/cmake/MesosTestsConfigure.cmake 48b4d2fd74b545c03162c7cb06013baf67298241 
>   src/usage/CMakeLists.txt fc87b297832157b679039c68baf9ddc68a292060 
> 
> 
> Diff: https://reviews.apache.org/r/61753/diff/1/
> 
> 
> Testing
> -------
> 
> Tested with `make check` and `ninja-build check` on CentOS 7. All tests pass. Tested with libevent and OpenSSL, all tests passed. Tested with Java, all tests passed (the "example" Java tests which depend on a target that's not yet enabled).
> 
> Tested on Windows with and without Java, all tests passed. With Java requires some environment setup which will be documented.
> 
> Tested release configurations, all tests pass.
> 
> 
> Thanks,
> 
> Andrew Schwartzmeyer
> 
>