Creating an improved CMakeLists.txt file

[jrsnen]

This issue is a compilation of all improvements suggested for current CMakeLists.txt file. Having all the issues collected under one issue makes managing changes easier and issue list of uvgRTP clearer.

Existing unresolved issues:

• Separation of public and private header files #12 Creation of subfolder uvgrtp in the include folder
• Don't use absolute path as install destination in CMakeLists.txt #13 Not using absolute install destination paths
• Also list header files in CMake #37 List header files
• Add target_include_directories PRIVATE to Cmake and remove relative paths from includes #38 Adding PRIVATE to target_include_directories

Additional suggestions:

• Test integration
• Static code analysis
• Better install integration

@db-tech has expressed interest in doing a first draft of the file. This would be greatly appreciated. I'm not currently working on any of there, but if there is anything I can do to help, feel free to ask me. I'm only starting to learn CMake.

Also if someone has further improvement ideas, mention them in the comments and I can add them this issue. These improvements can be made in small patches or in one big file change. Whatever suits best. Additional improvements in file are also welcome as long as they don't unnecessarily complicate the usage of uvgRTP or CMakeLists file.

[db-tech]

Looks good to me so far. Will propaply take Some days because of some work related stress at the moment but it will be done.

[jrsnen]

Looks good to me so far. Will propaply take Some days because of some work related stress at the moment but it will be done.

Hopefully stress isn't too bad. Sounds good that this will get done. No need to hurry with this, especially since I will be focusing on writing a paper for the next two weeks.

When the paper is finished I will try to create a more comprehensive plan for updating uvgRTP based on what has been said thus far so there is common understanding where the project is headed.

[jrsnen]

I'm sorry, I didn't communicate this earlier, but 3b5acb7 contains (mostly untested) implementation for the subfolder include move. I needed it for my example improvements.

[db-tech]

yeah, the merge was a bit of a mess ;-)... But I get why you cannot simply hold still until I finish this task.
But it would be cool if you could hold changes to the CMakeLists.txt and the general structure to a minimum for some more day :-)
I will create a PullRequest or some kind of preview to the new CMakeFile.txt next week.

[jrsnen]

@db-tech Sorry about that, I should have asked first. I could have maybe focused on some other task first than the examples.

This week I will be focusing only on the examples and fixing them. Apart from maybe a small change to lib.hh, there should be no modifications outside the examples.

I also have some school work I should be doing.

[db-tech]

@polusto don't worry... it may have sounded worse that I meant it to be. Not that big a deal :-)

I have another question.
How do you want the Versioning to behave ?

Those are the options as I see them at this moment:

1. Just define the version in the "project()" call.
   • Because this has to be the complete version (major.minor.path), this would mean that with every release you have to change the CMakeLists.txt file
2. I could create a metafile and put the version there.
   • This essentially has almost the same drawback, but you only would had to change this metafile instead of the CMakeLists.txt
3. Get the version from some git properties (like tag+commit hash).
   • This has the benefit that you don't have to change source to change the version but this has the drawback that you cannot see the version in the source directly.

Maybe you have some other Ideas ? it can also be a combination of things !?

[jrsnen]

First of all, any improvement compared to current version are accepted. If some parts seem too much work, feel free to omit them.

We had a short discussion on this matter and the consensus was that the best way to do this would be to a mix of 1 and 3. We would want users who use uvgRTP outside of Git to have some kind of version, but if they use git version, it would be nice to also see the commit hash they are using so fixing their issues becomes easier. Also it would be nice to have the date of the commit/version visible if possible. This would mean having the version in CMake, but also using some sort of method for getting the current hash and date from Git if it is available. We also utilize tags in git, which could be used to remove hash from those that match exactly the version tag.

It would probably be nice to have an API call in lib.hh to ask for version and also print the version when using the library for the first time (or maybe somewhere like creating session).

So something like
Git:
uvgRTP Version 2.0.0 - "06fdcd519bc62ac4d01df7692ebd12b8205afcff" (2021-06-24)

Git (exact release version):
uvgRTP Version 2.0.0 - Release (2021-03-23)

Non-Git:
uvgRTP Version 2.0.0

There may still be some improvements that could be made, but this is my first suggestion.

[jrsnen]

@db-tech thank you for the question by the way

[db-tech]

We are currently working on it and we have some discussions about the Non-Git part.
I am not sure if this scenario is even valid or important.
Do you know a scenario where it is useful to have the sources where the .git part is removed ?

Also, I am not sure how the releases should work here in general and with github.
unfortunately I've never used Github releases and actions before.
Do you use Github actions to build the release automatically ?
I am not sure how to distinguish between a manual build and a release build automatically ( if we even can...)
Usually I use jenkins to build the releases, so it would be nice if you could give me some insights into the process here.

The CMake stuff should be pretty simple, I just need some more information.
Thanks!

[jrsnen]

We are currently working on it and we have some discussions about the Non-Git part.
I am not sure if this scenario is even valid or important.
Do you know a scenario where it is useful to have the sources where the .git part is removed ?

The first scenario that comes to mind for non-git uvgRTP is Linux distributions adopting uvgRTP, where they don't include the git tree in their distributions. but a version should exist.

Also, I am not sure how the releases should work here in general and with github.
unfortunately I've never used Github releases and actions before.

With the release thingy (not that important feature, just adds extra clarity where available) I was talking about just using git tags if it exists for current commit. With GitHub we usually push a tag to repo and then create the release in GitHub connected to that tag. Github has no part in this versioning and this feature would not work if the repo does not have the tags.

Do you use Github actions to build the release automatically ?

That would be probably be useful. @fador has mostly handled these sorts of things for us so far. He setup CircleCi to automatically test at least the newest branch of master. We also have some plans to test uvgRTP more extensively using Github Actions using our own server, but with all these we have to take care that no mining can occur with outside branches.

I am not sure how to distinguish between a manual build and a release build automatically ( if we even can...)

Git tags? I probably wouldn't mix outside build systems like Github actions with setting release flag on or something although I guess it could be done.

Usually I use jenkins to build the releases, so it would be nice if you could give me some insights into the process here.

Honestly, everything is currently done manually since we haven't had that many releases :D

The CMake stuff should be pretty simple, I just need some more information.
Thanks!

Thank you for making uvgRTP better!

[jrsnen]

Also, I totally forgot the Qt project from my considerations (which is somehow still a supported build system). There probably should be at least some version present there as well. Maybe we can handle that separately after the CMake solution works.

Maybe as a first solution could be that if none of the presented methods works, uvgRTP would print
uvgRTP version unknown

[db-tech]

The first scenario that comes to mind for non-git uvgRTP is Linux distributions adopting uvgRTP, where they don't include the git tree in their distributions. but a version should exist.

Not sue what you mean by that. Delivered Packages would contain the binaries and includes, not the source. So this would of course be versioned anyway. But maybe we just talk past each other :-/
Because I am on vacation tomorrow for a week, I would skip the Versioning stuff for now and just use the CMake version.
Maybe it would be best if we discuss this topic in person when I am back from my vacation ?

Nonetheless.. I will push what I have so far sometime this evening, so you can take a look.

[jrsnen]

You are probably right, maybe it would be possible to just set the variable build time and use that in this case.

Another case that came into mind is the download source button in GitHub release (or download ZIP in front page on GitHub) that does not contain the git version history, although I don't know how popular that way of building uvgRTP is. What I'm maybe trying to say that it is possible that uvgRTP source code exists without git and that should do something functional, even if it is not the typical usage scenario. This can be handled afterwards also and for now a version in CMake file would be enough.

As for pushing code, whatever suits you best. I'll probably start my vacation in August, but we can have a call before that if needed. I'm not in a hurry with this branch, since I have plenty to do before my vacation already and I will most likely not complete everything in any case. For example, I should also probably take a look at RTCP since it is causing problems for users.

[db-tech]

Hi,

I am back from my vacation and back to the CMake stuff.

I've pushed a version on my fork (https://github.com/db-tech/uvgRTP/tree/newCmakeVersion) that is still work in progress, but should be enough to get a rough understanding where I meant to go with this.
I suggest that we have a meeting soon and discuss some of the open topics.

Now I will add some description about the CMake internals and the intend behind it.
I will put the line number in front of each explanation

CMakeLists.txt

1: 3.12 has some nice features I would like to have. But I would also suggest to not fall behind the newest versions to much, because CMake can simply be downloaded and built.

10: Creates an external file for project details, e.g. version, so if we change the version, we do not have changes in the CMakeLists.txt

17: I usually use an external cmake file for the dependency stuff (More details later)

18: This file is responsible for creating in source access to the versions (More details later)

22: Setting the versions of the library itself. This will use the version set in in the project and (if available) the git hash. As I said, this is not the final implementation and I would like to talk to you guys before going any further.
If you set the version to 2.1.0, the following files will be generated in linux:

- libuvgrtp.so.2.1.0
- libuvgrtp.so.2 -> libuvgrtp.so.2.1.0
- libuvgrtp.so   -> libuvgrtp.so.2.1.0

Or if the git repo is available:

- libuvgrtp.so.2.1.0-f254c86
- libuvgrtp.so.2 -> libuvgrtp.so.2.1.0
- libuvgrtp.so   -> libuvgrtp.so.2.1.0

68: Here I added the headers as requested. Of course that might change soon because of changes of header visibility and placement, but for now this should be fine.

93: Adding the dependencies. Here I added the version library, but more on that later

100: Defining included directories dependent on the type of usage
- 101: Public headers when building the project itself OR added the project as part of another project
- 102: PRIVATE headers when building the project
- 103: PUBLIC header when installing the library and using it in another cmake project (Example is following lateR)

106-110: Some compiler features, I googled to find useful stuff for windows, for linux that should be fine. I would suggest to add the "-Werror" at some point, but that is not possible at this moment because there are to many issues in the source at the moment.

116: Should meet the same requirements as before but removed necessary lines

126: add test directory. For now, I just created an example test case testing the versioning library. That is of course just a showcase of how to simply create unit tests.
This test directory itself contains a simple CMakeLists.txt file. That file is Work in Progress. I will add cmake specific test target, test execution results and so on later!
But for adding tests, that should be a good start.

Installation (132-189)

132: Here the installation stuff begins. First, I added GNUInstallDirs for some useful variables

133: Set the Installation properties of the library. This should work in linux and Windows but is still missing mac-os specifics. I can add that later!
The COMPONENT Settings are useful when packaging the library (e.g. debian packages). Then we can create runtime and development packages. packaging is still missing,
but I prepared for it anyway.
The EXPORT property is just to connect this target installation setting with the export installation later on.

145: collect the header files that should be installed
146: Define the header install destination. I added another COMPONENT here that can then later be added to the development package

152: This will create a cmake version file that can later be used to uses versions in find_package(...VERSION...)

158: This will create a target file directly to the build tree. This can be useful for example, if uvgrtp us used in a sub-build for another system

164: Create/Copy a Config.cmake file to the install directory to add the external dependencies (e.g. THREADS). Still work in progress but should work so far.

170: Added the MAcros file just for completeness, it's empty for now.

179: Now we create and export the "...Targets.cmake" file and Define the namespace uvgrtp:: that can than be used in add_link_libraries().. (example at the end)
The ...Targets.cmake file should contain everything needet to use the project or installed library in another cmake project.

185: install the ...Config.cmake, ...Macros.cmake and the ...ConfigVersion.cmake file that cmake will create for us

FindDependencies.cmake

4-6: Use Cmake to find the available threading library

11: finding git. We have to discuss if we want this to be a requirement or not and what the resulting actions should be.

16-30: Fetch GTest if this is not found. At the moment, the Gtest will be downloaded and the tests will be added in any case. It might be better to have a build option to disable/enable testing for users.

Versioning.cmake

1-15: creating the short-hash version part. atm, This only happens if git was found and git repo is available

17: Here we copy the replace file of rtp_version.cpp.in with rtp_version.cpp and replace all the version details at build time.
The reason I build a library OBJECT library for that is so we do not have to rebuild everything that includes the rtp_version.h every time something changes

Installation example (Linux only)

So in order to install the library you can just do make install and it should be installed in the appropriate directories.
What I usually do is to specify a my own installation directory for the libraries my project needs. This could simply be done with:

cmake .. -DCMAKE_INSTALL_PREFIX="/my/target/dir/"

If we then want to use THIS installation in our cmake project we could to something like this:

CMakeLists.txt

cmake_minimum_required(VERSION 3.19)
project(TestExternalLib)

set(CMAKE_CXX_STANDARD 17)

# Version could be 2, 2.1, 2.1.0, 2.1.0-hash or just left empty)
find_package(uvgrtp 2.1 CONFIG REQUIRED VERSION )


add_executable(TestExternalLib main.cpp)

target_link_libraries(TestExternalLib PRIVATE uvgrtp::uvgrtp)

main.cpp

#include <uvgrtp/lib.hh>

int main()
{
    uvgrtp::context ctx;
    return 0;
}

We just have to define the directory where the cmake uvgrtpTargets.cmake is installed in the cmake command line

cmake .. -Duvgrtp_DIR="/my/target/dir/lb/cmake/uvgrtp"

Still missing/open points:

• MAC-OS specifics
• Packaging
• Versioning (git requirement)
• Test Project details
• Static code analysis

[jrsnen]

@db-tech Thank you for this branch. I sent you an email about a possible meeting. There are few minor changes that I would like to discuss. We can discuss them in the meeting or I can add them here if you prefer.

In any case, great work. This is a great step in uvgRTP development.

[db-tech]

Hi there, i have an additional question about the versioning.

Would you like to have the git hash as a default? then we could add a cmake parameter for releases e.g.: cmake .. -DRELEASE_VERSION=TRUE and omit the hash.

Or would you rather like to omit the hash by default and add this likewise with a cmake parameter, e.g.: cmake -DWITH_HASH=TRUE

Or any other ideas ?

[jrsnen]

I would have the hash there by default, then have the release parameter add text "release" somewhere to make it feel special and distinguish it from not having git at all.

So something like:

Git with hash:
uvgRTP version 2.0.0 "06fdcd51"

Release:
uvgRTP version 2.0.0 release

No Git:
uvgRTP version 2.0.0

Not sure about the best way to format the lines.

[db-tech]

So, at last I've added some packaging functionality.
We can build rpm, deb, tar.gz and some other for linux.
For windows I've tested to create an install with nsis:
(Sorry that the screenshots are in German :-) )

PR is open.

I've could had added more features, but thought it would be better to keep it that way for know and add stuff later (like the static code analysis)

I did left some files empty to be filled by you guys (like some packaging files Description.txt, Licence.txt....)

[gjaegy]

I get the following error when trying to generate using CMake on windows:

CMake Error at CMakeLists.txt:21 (set_target_properties):
set_target_properties called with incorrect number of arguments.

[marco-tranzatto]

@gjaegy maybe you're interested in PR #100 as it should fix the error you encountered.