search

CesiumGS / cesium-native

Apache License 2.0
407 stars 209 forks source link

Review the CMake- and build infrastructure #78

Closed javagl closed 3 years ago

javagl commented 3 years ago

First a disclaimer: I'm not a CMake expert, and not an expert for the setup, structuring, building, installation, dependency management, and deployment of C++ projects. But there are some things that caused me to raise an eyebrow when getting started. A such, this is more of a brain dump - and although I try to structure this sensibly, some of these points are strongly related to each other. Responses to some of these points may vary between "That's just the way it is, get over it" and "Yes, that will be changed, and the progress for doing so will be tracked in issue XYZ", but I want to mention them, at least.

(Note: It might be that the "root cause" of the first points is mentioned in the last point, namely, that there are "No installation options", but I'm not entirely sure about that)

  1. The use of nested submodules.

Right now, cesium-unreal-demo contains cesium-unreal as a submodule. And cesium-unreal contains cesium-native as a submodule. And cesium-native contains all external libraries as a submodule. I see the advantages of submodules, particularly for third-party libraries, and to some extent even for ~"dependency/version management" during the development of in-house libraries. But it has some drawbacks. (FWIW, I have created a .BAT file locally, to do a cd cesium-unreal-demo/Plugins/cesium-unreal/extern/cesium-native...). One technical limitation is that it is essentially not possible to have a directory structure like

When having a branch featureX in cesium-native, and a matching branch featureX in cesium-unreal, keeping the checked-out versions and submodule versions in sync is a bit clumsy.

  1. The project/build is huge (and slow)

Right now, when checking out and building cesium-native, the resulting Visual Studio solution contains more than 40 (forty) projects. The build takes a considerable amount of time (at least, much longer than the "core" cesium-native build would take, which consists only of 3DTiles, Geospatial, Geometry, Utility, and Async). Every change in the CMake file will trigger a full reload/rebuild of all libraries.

The thing that makes this look odd for me is: It's building all dependencies, each time. For example, the "draco" libraries. They are checked out in one version. We do not modify anything there. They will hardly ever change. They should be built once, then installed, and the result should simply be used by cesium-native.

  1. The project structure in Visual Studio

One result of 2. is that it's becoming increasingly hard to use Visual Studio sensibly. For example, searching for a certain function name takes roughly 30 seconds (because it's searching in the whole solution, and not only in the cesium-native core libraries). The list of "Header Files" in the solution explorer for Cesium3DTiles contains roughly 150 files, and many of them are not header files of Cesium3DTiles.

(FWIW: I usually have an Explorer and TextPad open. The Explorer shows the actual header files. TextPad does a full-text search in ~1 second. But this should not be necessary).

  1. Dependency management in CMake

CMake is pure anarchy. There are always 50 ways to achieve the same thing, and usually, 49 of them are wrong in one way or the other. The fact that there are no "best practices" is annoying, and the fact that "established practices" seem to have changed significantly between CMake 2.x and 3.x makes it even harder. I've used CMake 2.x a while ago, and now started reading, for example, https://pabloariasal.github.io/2018/02/19/its-time-to-do-cmake-right/ , as an update. But it's not trivial to sensibly apply this in practice. However, if someone wants to use, for example, Cesium3DTiles, then he should be able to do this by adding

find_package(Cesium3DTiles 1.0 REQUIRED)
target_link_libraries(example Cesium3DTiles::Cesium3DTiles)

to the CMakeLists.txt and be done. There should not be the need to add cesium-native as a Git submodule. There should not be a need to do some add_subdirectory. It should not be necessary to manually set any Cesium3DTiles_INCLUDE_DIR variable.

  1. Compilation settings in CMake

The way how the third-party libraries are currently integrated via CMake makes it hard to apply the high warning levels that should be standard for the Cesium libraries. I'm not familiar with the way how things like that are done in CMake 3.x nowadays, but the classification of dependencies, headers, or target_compile_features as SYSTEM, PUBLIC, PRIVATE seem to offer some options to potentially solve that more cleanly.

  1. No installation options

This may be the root cause of many of the aforementioned points. The workflow/structure that I knew until now was that there was a library (in-house or third-party). This had some CMake files that could be used to create the Visual Studio project. The project then had an INSTALL target that could be executed, causing the (compiled) dependencies and headers to be put into some thirdParty/Debug/xyz directory, with include and xyz.lib being there, ready to be picked up by find_package. The actual VS solution that was actively developed then consisted of a single project, with all the dependencies ready and in place. I have never set up such an "INSTALL" infrastructure on my own, though (only "copied+pasted" the specific parts that have been required for new projects), and all this was with CMake 2.x, and the way how this is set up may have changed considerably since then.

javagl commented 3 years ago

In order to be able to install cesium-native without having to install asyncplusplus, draco, glm, GSL, tinygltf, tinyxml2, uriparser and spdlog first, the installation should also install the transitive dependencies. According to a quick websearch, this might not be possible (or at least, not easy) (e.g. https://stackoverflow.com/a/52110751 ), but maybe there is a solution

jtorresfabra commented 3 years ago

If the case is to compile our own third party and install, the usual route for that is to use External_project_add (https://cmake.org/cmake/help/latest/module/ExternalProject.html). It is able to download from github/svn/whatever and compile and install projects that are not even using cmake. I rather to use that instead of adding submodules for thirdparty. (Not sure the discussion on why is relevant here).

If you want to install precompiled binaries depending on the platform then that's a different case, there are tools like conan(https://conan.io/) to handle big projects with a lot of dependencies. But in the current scenario I think we should be safe just downloading and compiling what we need, as cesium-native is still a small library (and hopefully it will stay as-is).

kring commented 3 years ago

No longer part of this initial release because we're not going to make a big splash about cesium-native initially, and this isn't needed for Cesium for Unreal.

Samulus commented 3 years ago

If the case is to compile our own third party and install, the usual route for that is to use External_project_add (https://cmake.org/cmake/help/latest/module/ExternalProject.html). It is able to download from github/svn/whatever and compile and install projects that are not even using cmake. I rather to use that instead of adding submodules for thirdparty. (Not sure the discussion on why is relevant here).

If you want to install precompiled binaries depending on the platform then that's a different case, there are tools like conan(https://conan.io/) to handle big projects with a lot of dependencies. But in the current scenario I think we should be safe just downloading and compiling what we need, as cesium-native is still a small library (and hopefully it will stay as-is).

vcpkg is anothr C++ package manager supported by Microsoft and they finally added support for setting dependency versions, it might be worth considering too. I liked it last time I used it for cross platform development.

kring commented 3 years ago

Some of this has already been done. I wrote some more specific and actionable issues for the remainder and am going to close this one:

230

231