Rewrite CMake configuration #1275
Merged
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
johnbartholomew
force-pushed
the
rewrite-cmake
branch
from
c41e3b4 to
c7fc73e
Compare
Using GoogleTest just makes it more uniform with other tests so it can be added to Bazel and CMake test suites. However, unfortunately it means removing the test from the Makefile test set.
johnbartholomew
force-pushed
the
rewrite-cmake
branch
from
c7fc73e to
a895852
Compare
Contributor
|
Nice, I have to replace it when importing this project in Clion |
Minimum CMake version is increased to 3.15. That is chosen based on guidance from here: https://cliutils.gitlab.io/modern-cmake/README.html#shouldn-t-i-support-the-oldest-version-possible Notes on this: * Global variables are barely used at all now. There is one to hold the jsonnet version number. * Most config is set through target properties. * Target dependencies are cleaner. * Reduced use of CMakeLists.txt in subdirectories. This isn't strictly better it's just my personal preference. * The generated file std.jsonnet.h is put in the CMake output dir, instead of being written back to the source directory.
The CMake test step runs the regression test which is super slow if the jsonnet binary has been built in Debug (no optimisations) mode. And using Ninja means that we can exploit multiple cores on the CI action runner, if it has multiple cores, without needing to do extra stuff to find out a core count or correctly set a -j flag.
johnbartholomew
force-pushed
the
rewrite-cmake
branch
from
0090560 to
c25a96e
Compare
This works around an annoyance with gtest_discover_tests. The test discovery system works by running the test binary with flags to make it produce a JSON file that gives details of all the tests. However, that file is written to the working directory. If we set the working directory for the tests to be the source directory, then the tests can access their test data by relative paths, but the JSON test details files for test discovery get written to the source directory, which is ugly. So, we instead provide the location of the source tree through an environment variable, and then in the tests themselves we change the current working directory to the source tree directory. Why change CWD instead of just using absolute paths for input files? Because error messages produced by jsonnet include the file paths, and we need (or at least want) those to still be relative paths so that we can compare against stable golden outputs.
johnbartholomew
force-pushed
the
rewrite-cmake
branch
from
c25a96e to
5f0ea43
Compare
This was referenced Feb 1, 2026
Closed
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Minimum CMake version is increased to 3.15. That is chosen based on guidance from here:
https://cliutils.gitlab.io/modern-cmake/README.html#shouldn-t-i-support-the-oldest-version-possible
Notes on this: