Migrating all dependencies to vcpkg

[jherico]

This is based off my original work in the now closed PR #644 which contains a bit of discussion on the topic.

This PR represents a first step to making Cesium-Native available in vcpkg as a resource for others to fetch, inline with #248. It should also reduce the maintenance burden of managing all those additional projects and configs inside the CMake build tree.

Summary of the changes

Migrate of the /extern dependencies to be vcpkg based with the following caveats

• CSPRNG isn't available in vcpkg, but examination of the source code suggests that it's only being used for random token generation where a PRNG should be sufficient. I've replaced the code in fillWithRandomBytes.cpp with a pure C++ version using the STL.
  • if there's a strong argument for something stronger, I can look at improving the seed value, thought it should be noted that the previous UWP version seemed perfectly happy depending on a PRNG.
  • Also the backing API CSPRNG uses on Windows is now deprecated.
• s2geometry is available in vcpkg, but only for non-windows platforms for some reason. cesium-native is building some custom subset of the codebase to avoid the OpenSSL dependency. Given the choice between trying to fix the vcpkg port to work on windows and leaving this dependency in I opted for the latter. For now.
  • If Cesium decides to accept this PR and go this route, it would be pretty simple to fork the main VCPKG repository and put the customized version of s2geometry into the fork, which would eliminate the last /extern project dependency.

Modify the CI config to cache the vcpkg build artifacts

The vcpkg macro I've introduced builds the dependencies outside of the CMake buildtree (by default in ~/.ezvcpkg/<commit id>) and thus they're easy to cache. This makes the aggregate cost of building your upstream dependencies on fall to zero both for CI builds and for developers after they've done a local "clean" because the external artifacts aren't built by the resulting Makefile / IDE project. Because the dependency buildtree is also tied to the commit ID in VCPKG, It also means if developers want to try out new versions of specific dependencies, they can switch between those new dependencies and the current dependencies without having to rebuild them each time.

The current setup uses the project root CMakeCache.txt as a proxy for "dependencies have changed" so some false cache misses may still occur. This could be fixed by pushing the vcpkg call into a vcpkg.cmake file that gets included, but this would require a little extra work to ensure that the ezvcpkg_fetch exported variables like EZVCPKG_DIR and CMAKE_TOOLCHAIN_FILE were then re-exported to the root scope.

You can see the impact of this change comparing an existing run from your current main branch with this PR branch build where the cache was fully populated. I'm uncertain why the Visual Studio builds aren't seeing much benefit (not sure why), but the overall CI time has at least gone down by ~8 minutes, mostly due to longest running job MacOS now running substantially faster.

This change also eliminates the need for a target_link_libraries_system macro. The issue here was that include paths inside the CMake root folder get treated as non-system by default. By moving to vcpkg, and an out-of-tree build for the external dependencies, this problem goes away for everything except s2geometry, which can be handled explicitly by adding the SYSTEM directive to the target_include_directories declaration for that one project.

Because I'm depending on static libraries, I needed to set the set CMAKE_MSVC_RUNTIME_LIBRARY to MultiThreaded / MultiThreadedDebug to avoid runtime conflicts with the static external libraries we're linking, otherwise the main project uses MultiThreaded DLL by default and causes link time errors. I'm currently only trying to target a static library version of Cesium-Native, but I suspect that with a bit of work both a shared and static version can be supported.

Changes / fixes required because of version differences

In a few cases, the version of a library in vcpkg was significantly newer than the one being used by Cesium, and rather than try to create a custom vcpkg fork right off the bat, I decided to try to deal with any problems caused by those changes inline.

• I encountered an error with the fmt formatting of the rapidjson error enum, and had to add a specialization of the fmt::formatter struct for the rapidjson::ParseErrorCode type to correct it. I'm uncertain if this is triggered by a version difference in fmt or rapidjson or what, but the addition of a CesiumUtility/Log.h header that includes the template specialization fixed the issue.
• Similarly I encoutered an error in validateDracoMetadataAttribute with an implicit signed/unsigned conversion, which is probably traceable to some change in the draco library version. I fixed it with a simple static cast, since it appears to be something that will be in the range of 1-4. This could also probably be fixed by changing the signature of the receiving function, but I didn't want to try that initially as I didn't know what else might depend on the existing signature.
• Added a cactch.hpp helper header to bridge the API difference between the prior extern version and the current vcpkg version of Catch2.
• Added a cesium-sqlite3.h header to replace the one removed from the sqlite3 checkout
  • An alternative would be to just include the sqlite3.h header on the assumption that the dependency will always be coming from vcpkg, thus eliminating the need for a preprocessor macro. I'm not sure what the current use case is for supporting an alternative sqlite3 version would be unless some customers are already linking it and want to use the same version. For the static build this shouldn't be a problem though.
• A few of the unit tests for S2CellId were failing after the update to the vcpkg version, but I determined that the tests themselves were invalid:
  • One was validating that a cell centered on the international date line had a longitude of -180°, but in fact either -180° or 180° are valid interpretations. Fixed the test to reflect that by taking the absolute value of the cell center longitude.
  • Two tests were validating the longitude of cells that were centered on the pole, which doesn't make any sense, as the poles are singularities for longitude. Disabled the tests by commenting them out and added comments as to why.

[jherico]

@kring I managed to get a PR merged to s2geometry that fixes the windows builds and I have another PR that updates the vcpkg version of s2geometry.

Once that's merged I should be able to update this PR to make all dependencies vcpkg based.

[kring]

Awesome, thanks @jherico! We're slow to review this, but we haven't forgotten about it. Thanks for all your work on it!

[jherico]

@kring this is perplexing. I managed to get s2geometry updated in vcpkg so that it builds on windows, and integrated that into the PR, moving the last submodule based dependency to vcpkg, BUT for some reason on the vs2019 version of the CI server the package built package isn't found by CMake. I don't understand though why the CMake binary would behave differently because the target version of Visual Studio is different.

The only other problem I encountered was that one of the unit tests was failing because it seems to depend on the semantic meaning of certain s2 cell IDs, and at least one of them has changed.

I'll see if I can look closer at the VS2019 logs to try to determine why it's failing in the next couple days.

[kring]

Huh I'm not sure @jherico. Could the GH Actions runner perhaps be caching something vcpkg-related? Total guess.

because it seems to depend on the semantic meaning of certain s2 cell IDs, and at least one of them has changed.

I'm really surprised S2 cell IDs would change. I don't see any mention of that in the changelog, at a glance. We'll have to investigate further.

[jherico]

@kring I think I figured it out. This is an image from the page on the cell hierarchy:

Cell 5 is the one that's centered on the south pole, and the failing test was the one checking the longitude, which is basically a meaningless question. I've commented out this test as well as another test for cell "b" which is centered on the north pole, but is also checking the "longitude". This one wasn't failing, but again, it's a meaningless check and the implementation could change again, so this should avoid regressions (the latitude check for both is still there and the old tests are still present, but commented out with a note as to why).

There's another test that validates a cell centered on the international dateline, which evaluated as -180 degrees, but now is 180. I've updated that test to use the absolute value of the cell longitude to prevent further content-free regressions.

[jherico]

Man, I really did not want to have to install VS2019 today, but here we are. The VS2019 failure is because the s2geometry source code uses a "terse" static_assert, which is technically a C++17 feature, but s2geometry is set to C++14. Every compiler except VS2019 doesn't care and allows terse static_assert anyway, but if VS2019 is a required target for cesium-native then I have to make sure that s2geometry is compliant. I've got PRs for both s2geometry and vcpkg that should fix this. I'm attempting a build of cesium-native off my vcpkg fork to see if corrects the problem as expected. This also gave me some insight into some shortcomings for my ezvcpkg script which I've addressed in the embedded version in this repo and will work towards integrating into my main ezvcpkg repo.

[jherico]

[azrogers]

Thank you so much for all your work!