Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add CMake build system #1324

Merged
merged 3 commits into from
Feb 9, 2024
Merged

Add CMake build system #1324

merged 3 commits into from
Feb 9, 2024

Conversation

ryandesign
Copy link
Contributor

Here's an initial version of a CMake build system. I've left some TODOs in the CMakeLists.txt which I'll look at next but I think it might be enough to merge already. Of course, let me know if there's anything you want changed.

It successfully builds the SDL version for me locally on macOS 12 and Linux Mint 21.2, and in GitHub Actions on macOS 12 and Ubuntu 22.04.3.

My intention is that this would replace the SCons build system (see #1275) but you may want to give users time to transition before removing the SCons build system. For now, I've updated the CI workflow to build using both SCons and CMake.

In the SCons and QMake builds I see you used globs to accumulate all the *.cpp files. CMake can do this but I've read it's not recommended. One reason given is that if you don't modify the CMakeLists.txt file (or a file it includes), CMake won't know that it should rescan to find new files. Therefore I've listed all the *.cpp files individually. I've put them in a separate file, cmake/CLK_SOURCES.cmake, so as not to clutter up the main CMakeLists.txt file.

I wasn't sure whether you preferred to edit the file list manually or have it done automatically. The bash script cmake/generate_CLK_SOURCES can be used to regenerate cmake/CLK_SOURCES.cmake. Depending on how long the SCons build system sticks around, the script could be modified to update the file list in the SConstruct file as well, and conceivably the QMake clksignal.pro as well.

Do you remember why the SCons build links with libpthread? It was added with a lot of other stuff in c0055a5 but it builds fine without it on macOS and Linux Mint and Ubuntu, so I didn't add it to the CMake build, but I can if it's needed.

It's customary to list the project version in the project line of the CMakeLists.txt, so I have. That makes one more place where the version would need to be updated each time. Later, I'd like to improve things so that there's only one place where the version number needs to be set, such as a simple VERSION text file that each of the build systems could read and use.

Where the SCons build used -Wall, I've used -Wall -Wextra in the CMake build. This shows a few warnings currently that you might want to look at to see if they're important. Might want to add -Wpedantic as well but that shows a lot more (possibly irrelevant) warnings with your current code.

The SCons build used -O2 optimization flags. The CMake build does not specify them and leaves it to the user to select the build type they want. For example, -DCMAKE_BUILD_TYPE=Release uses -O3 or -DCMAKE_BUILD_TYPE=MinSizeRel uses -Os.

I haven't documented the CMake build process here yet; I would do that after #1323 is dealt with.

@ryandesign
Copy link
Contributor Author

Oh! And to reiterate, I'm a beginner at writing CMake build systems, so if I've done something dumb or not in the most CMakey way, let me know.

@TomHarte TomHarte merged commit 472297e into TomHarte:master Feb 9, 2024
5 checks passed
@ryandesign ryandesign deleted the cmake branch February 12, 2024 19:23
@ryandesign
Copy link
Contributor Author

Do you remember why the SCons build links with libpthread? It was added with a lot of other stuff in c0055a5 but it builds fine without it on macOS and Linux Mint and Ubuntu, so I didn't add it to the CMake build, but I can if it's needed.

On macOS, libpthread has always been part of the C library and libpthread.dylib is nothing more than a symlink to libSystem.dylib for compatibility with Linux software that unconditionally uses -lpthread. On Linux, libpthread was a separate library at the time that -lpthread was added to the CLK SCons build in 2017, but I just found out that libpthread was combined with the Linux C library glibc in 2021 which is why -lpthread isn't needed on recent Linux. For compatibility with older Linux, I should add pthread detection and use to the CMake build.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants