SDRPlusPlus
SDRPlusPlus copied to clipboard
AlexandreRouma
Restructuring and modernize CMake project
I'm willing to clean up the CMake project, and I started working on #290. As requested by @AlexandreRouma in https://github.com/AlexandreRouma/SDRPlusPlus/pull/290#issuecomment-903104274 I opened this issue.
IMHO improving the cmake structure of the project will improve the overall quality and the maintainability.
It is a work that would need different steps, but in this initial one my target is just to clear up a bit the project without doing any dramatic change from the developer point of view and possibly simplify the packaging/build phase.
Since it requires a quite relevant effort, if this kind of work is not welcome feel free to close this issue and I will stop investing time on this.
Just write down things that you think should be changed and how they should be changed and I'll consider it. It's much easier than going through one giant PR and picking out stuff to roll back.
It is a very generic and extensive work the one I'm proposing, it's not easy to pick up isolated things. But, I understand your point, I will try to summarize main points. What I was trying to achieve:
- CMake modernization: this involves every single
CMakeLists.txtin the project. Currently the project does not really uses CMake targets, which will help a lot in the overall management of the project (e.g. dependencies, modularization, packaging). - Dependencies: currently the project embeds external dependencies without any clear separation between internal and external code. Keeping them updated (e.g. for bug-fixes) may soon become problematic.
Beside this, there are other things I'm still trying to understand how to tackle, and it would be neecessary to discuss with you how to address them. But this may become the object of another PR.
An example is about the modules file structure and management. By making clear the distinction between sdrpp_core and the modules, it will open to an easier plugin development (e.g. by improving the cmake it will be possible to easily install sdrpp also as a development library, that can be used for building not "embedded" modules).
Looking forward to your comments and ideas.
Again, I was just willing to help in fixing up the CMake, if you are not open to it, I understand it.
CMake modernization: this involves every single CMakeLists.txt in the project. Currently the project does not really uses CMake targets, which will help a lot in the overall management of the project (e.g. dependencies, modularization, packaging).
For cmake modernisation, I can't use anything after cmake 3.13, this is already badly supported (eg. Ubuntu 18.04 and deriviatives) so I've already had issues. I'm already using targets for a lot of things, just not basic stuff.
Dependencies: currently the project embeds external dependencies without any clear separation between internal and external code. Keeping them updated (e.g. for bug-fixes) may soon become problematic.
The only two depdencies I embed are spdlog and ImGui because they're both means to be included directly in code. Adding those as dependencies would just cause issues with distros that don't have them in their package managers. They're not as mainstream as FFTW3, glew and GLFW.
An example is about the modules file structure and management.
This is already planned (discussed in https://github.com/AlexandreRouma/SDRPlusPlus/issues/161)
CMake modernization: this involves every single CMakeLists.txt in the project. Currently the project does not really uses CMake targets, which will help a lot in the overall management of the project (e.g. dependencies, modularization, packaging).
For cmake modernisation, I can't use anything after cmake 3.13, this is already badly supported (eg. Ubuntu 18.04 and deriviatives) so I've already had issues. I'm already using targets for a lot of things, just not basic stuff.
Mmh, I think there is a bit of confusion. CMake 3.13 is more then enough for writing modern CMake, and current status of the CMake part of the project does not follow the basic guidelines (e.g. usage of global cmake variables, like in CMAKE_CXX_FLAGS).
Dependencies: currently the project embeds external dependencies without any clear separation between internal and external code. Keeping them updated (e.g. for bug-fixes) may soon become problematic.
The only two depdencies I embed are spdlog and ImGui because they're both means to be included directly in code. Adding those as dependencies would just cause issues with distros that don't have them in their package managers. They're not as mainstream as FFTW3, glew and GLFW.
I meant to use them as external project in the build tree (e.g. FetchContent): this allows to not have external and internal code mixed up in the same place like it is now, it eases the maintainability (e.g. think about updating the imgui folder, which currently is a mix of stb and imgui files), and it make a clear separation of concerns (e.g. each library target carries meta-information about how to compile/link it).
Moreover, together with the easier maintanability, it allows also to keep track of the dependencies. AFAICT from what I've seen till now, the dependencies "embedded" are:
- nlohmann json https://github.com/nlohmann/json
- stb https://github.com/nothings/stb
- spdlog https://github.com/gabime/spdlog
- ghc filesystem https://github.com/gulrak/filesystem
Eventually, falcon9_decoder module embeds libcorrect https://github.com/quiet/libcorrect.
An example is about the modules file structure and management.
This is already planned (discussed in #161)
Nice!
I am attempting to package this for Fedora. I've run into two concerns regarding cmake as of v1.0.3.
-
Overriding of CMAKE_CXX_FLAGS everywhere vs what rpmbuild (for the entire distribution) tries to set. For now I've done the very hamhanded trick of deleting all set(CMAKE_CXX_CFLAGS) throughout so that the rpmbuild definitions are used.
-
discord_integration.so plugin is attempting build to dynamically link (non-installed) discord-rpc.so because rpmbuild cmake macros set BUILD_SHARED_LIBS by default. That leads to discord_integration.so having a dependency on a file that isn't installed, and thus the final RPM having a Requires: libdiscord-rpc.so()(64bit) which can't be satisfied. discord-rpc should be built static and then linked into discord_integration.so. I've forced this now.
Furthermore, bundling of libraries is highly frowned upon in Fedora as one would expect. spdlog is available as a system library in Fedora, and should be used in preference to the copy in this source.
- I figured out how to remove rapidjson, spdlog, and fmt bundled libraries in favor of system libraries.
- resulting libraries belong in /usr/lib64 on 64-bit systems. In Fedora this is passed to cmake as LIB_SUFFIX which can be used in all the CMakeList.txt files, and is also needed in core/config.cpp generating the modulesDirectory setting.
https://bugzilla.redhat.com/bugzilla/show_bug.cgi?id=2003876 is the review ticket. https://koji.fedoraproject.org/koji/taskinfo?taskID=75641411 has a rawhide scratch-build. https://koji.fedoraproject.org/koji/taskinfo?taskID=75641416 has a Fedora 34 scratch build.
Overriding of CMAKE_CXX_FLAGS everywhere vs what rpmbuild (for the entire distribution) tries to set. For now I've done the very hamhanded trick of deleting all set(CMAKE_CXX_CFLAGS) throughout so that the rpmbuild definitions are used.
The reason they're completely overriden is to make sure the flags are present. I had a lot of issues before with it not applying -O3 which led to very bad performance
If you're gonna make it packageable for Fedora, please contribute a docker container (try to stick to the style of docker_build/ubuntu_hirsute) so that all commits can be built automatically to RPM.
Also make sure to follow the tables in the readme for which modules should be included and excluded in releases.
I pulled out nlohmann-json, stb_image, and stb_image_resize now as well in favor of system-provided libraries. I also restored your required CPPFLAGS in addition to those provided by Fedora. I'm also having to patch a good bit to remove all these libraries (patches applied inside the RPM spec file at build time). To the extent any rework of the cmake files can incorporate these or similar will make it easier to maintain in the distributions and then we can look at building in a container for your CI/CD process. https://domsch.com/fedora/sdrpp/fedora-34-x86_64/sdrpp.spec shows the steps, and patches are in that same directory.
You can see the built (S)RPMs here: https://koji.fedoraproject.org/koji/taskinfo?taskID=75694249. I am not building plugins for which the supporting libraries aren't already available, including airspy, airspyhf, bladerf, and plutosdr.
Packages for Fedora 34 and (upcoming) 35 will be in updates-testing tomorrow.
sudo dnf --enablerepo=updates-testing install sdrpp
These libraries are all meant to be directly in the code. Especially ImGui since they regularly push out breaking changes and SDR++ heavily uses custom ImGui widgets.
nlohmann json is also kinda weird to have as an externalk depdency when it's a single header file.... stb_image is given with imgui to handle image loading and manipulation.
As for the font, I'd rather not have to deal with every operating system's way to handle searching for fonts, not to mention that it can break the UI if the font isn't the exact size it expects.
just updated the cmake files to use add_compiler_option but I'm still having a few issues (especially with MacOS)
I'll have to verify that the flags are correctly being set on all platforms for maximal performance
Switched to a structure with mutliple categories instead of all modules in one folder
Alright and I've switched everything to use targets instead of per folder options.
closing as the structure is fine now. Only future change is global compiler argument variable to avoid code duplication.