CMake Targets carrying compiler flags

[t0m343]

This branch is implementing cmake targets to provide a proper export of the library using cmake.
The defines needed to compile against this library are now automatically exported when using CMakes find_package().

[tuexen]

@weinrank What do you think?

[weinrank]

I like most of the changes and think they're useful. 👍

But some changes have unwanted side effects, for example:
Moving some checks to the usrsctplib's CMakeList.txt will hide variables/defines from the programs CMakeList.txt
HAVE_SIN6_LEN will never be set in the program's directory.

[t0m343]

@weinrank the point of using CMake targets is, that CMake it self takes care of the compiler definitions.
Therefore CMake automatically defines all public definitions of a target whenever the target is linked to another target. Since all programs link against the lib and HAVE_SIN6_LEN is defined as PUBLIC definition in the Library target, this flag is automatically set for all programs.

[weinrank]

Disclaimer: I'm not a CMake professional! :)
Before I review the whole PR, I would like to discuss some changes.

You're right, HAVE_SIN6_LEN is defined for the programs, but the compiler options aren't.
I've mistaken the lines. 😖
The programs are built without the library's strict compiler options.

You distinguish between public and private defines.
But then you define both (private and public variables) as public (https://github.com/sctplab/usrsctp/pull/275/files#diff-97df4c89b4075d8e8dc8af124dde8b1bR275)?

From my point of view, only SCTP_DEBUG should be a public define.
INET, INET6, HAVE_SIN_LEN, ... should be private and not be defined outside the usrsctp scope.

If I understand the intention of your changes correctly, you want to compile/include the library independently of the programs?

[lgrahl]

From my point of view, only SCTP_DEBUG should be a public define.
INET, INET6, HAVE_SIN_LEN, ... should be private and not be defined outside the usrsctp scope.

My two cents: Every preprocessor definition used for a conditional exposed in public headers needs to be publicly exposed. Alternatively, the header needs to be generated.

For example, if INET isn't defined, accessing sctp_sockstore.sin is seemingly not possible for an application including usrsctp.h. This can result in awkward compilation warnings and broken autocompletion in IDEs.

[t0m343]

As @lgrahl already mentioned all definitions appearing in the header should be public to avoid an error caused by different configurations while linking against the library.

I did not want to define any definition private, since i wanted to be consistent with the current master. This is to avoid any possible errors for non public dependecies of your project or others relaying on this defines. So defining them private, as i would assume them to be, is definitely a possibility.

Further more, i did not touch the Windows only definitions since i currently do not have a running Windows System to test my changes.

So this commit is just a minimum change commit to enable CMake's target features.

[weinrank]

You're right in case of the INET6 and INET preprocessor checks in the exposed header.
I've created a PR #277 to remove the check from usrsctp.h.

Any specific reason why you moved the compiler checks to the usrsctplib folder's CMakeList.txt?

[t0m343]

Yes, i tried to separate the programs from the library. Since the library and the programs are compiled independently, they should also be handled as real sub-projects.
So the main CMake file only concatenates both projects.

[weinrank]

I tried the same idea of separating the programs from the library.
I came back to the monolithic solution where you can disable/enable the example programs via an CMake option.
The reason for this is the fact that the programs and the library share the same compiler options and most of the defines.
Separating the programs and the library will lead to code duplication and I don't see many cases where you would build the example programs but not the related library.
If I'm on the wrong track and the benefits of a separation outweigh: comments are welcome! 😊

If you agree, I'll cherry pick most of your suggested changes without your without separating the

[t0m343]

In this case i would suggest to keep the defines in the library directory and just make them public.
In this case the programs inherit the definitions from the library. And you still can add an if statement to decide if the definitions are public or private.
Furthermore, since the definitions are bound to the target there is no difference where you configure them. they have the same effect independent from the position where they are configured.

Currently all properties for each program should be the same as in your original script because i defined them as public property of the target.

[weinrank]

In case we set all definitions public, these definitions are also set in projects which depend on the usrsctp project by including it via find_package, correct?

If that's the case, this is not my intended behaviour.
Defines like "INET" and "INET6" are too generic to be exposed.

If you build the library without INET/INET6 support and try to use INET/INET6 features, the library will respond with an error indicating that the address family is not supported.

If you want those defines to be exposed, they should at least have an SCTP_ prefix.
IMHO the most elegant solution, as Lennart suggested, would be a dynamically generated header where those defines are set instead of using CMake magic.

[t0m343]

I think the solution, using generated headers, is a very clean way to solve the problem.
But in this case i do not see how you can ensure keeping the nmake build running.

[lgrahl]

IIRC the only define that is still used in usrsctp.h but not exposed is SCTP_DEBUG.

[tuexen]

Are we still needed anything from this PR or not?