CMake improvements: fix dll installation + add options to build examples/utilities + hide/export symbols if shared lib

[SpaceIm]

I've submitted edlib in conan-center-index, the public repository for conan packages (a C/C++ package manager): https://github.com/conan-io/conan-center-index/pull/3247

I had to patch a little bit edlib's CMakeLists and public header:

• shared build on Windows was broken (no import lib generated), because public symbols are not exported. This PR allows to export those symbols, as well as hiding others symbols with GNU compilers also.
• fix installation of dll file (should go to bin folder)
• add options to disable the build of example and app
• add edlib::edlib ALIAS target, so that consumers can link to edlib::edlib no matter whether they use add_subdirectory or find_package.

I've seen that meson was also supported. This PR shouldn't break meson build, but doesn't bring shared support for Windows if meson is used. I don't have enough knowledge of meson, but I guess that it shouldn't be too hard to add those modifications for someone else. if shared:

• at build time: hide all symbols + define EDLIB_SHARED and EDLIB_BUILD_SHARED
• at consume time: define EDLIB_SHARED

A more robust solution would be to rely on generate_export_header in CMake (generate a custom header at build time, with proper macro definition, to be included in public headers and installed with public headers), but I don't know if Meson has support for such feature, so I decided to go with manual macros.

[madebr]

The generated edlib.pc should have -DEDLIB_SHARED added to its Cflags when it is built as a shared library. Should the meson build system be updated as well?

[Martinsos]

@SpaceIm thank you for this PR, and very cool that Edlib is getting added to Conan :)!

I had Meson CI PR (#165) sitting for last week or so, waiting for people to give last comments before merging, and it is also doing some of the changes you have done -> exposing symbols from DLLs by adding macros to public header.

I wish I merged it sooner and avoided both of us doing the same thing, but I merged it now so you can work on top of that.

Please check again the current state of master (it is one new commit, from the mentioned PR) and update PR if needed (btw. this was my first time exporting stuff from DLL so if your macros/methods are better I am all for it), and I will find the time in the next couple of days to take a closer look at the changes -> you will hopefully manage to do this update by then.

Also, I removed CMake building from appveyor.yml -> that was probably a mistake, and you will want to add that back I believe.

[SpaceIm]

Looking quickly at your last commit in master branch, I would say that now static build on windows is broken (or will display a lot of warnings) because it always defines EDLIB_API as __declspec(dllexport) at build time and __declspec(dllimport) at consume time. I can fix that in edlib.h as well as CMakeLists, but I'll need some help for meson.build

[Martinsos]

Looking quickly at your last commit in master branch, I would say that now static build on windows is broken (or will display a lot of warnings) because it always defines EDLIB_API as __declspec(dllexport) at build time and __declspec(dllimport) at consume time. I can fix that in edlib.h as well as CMakeLists, but I'll need some help for meson.build

@SpaceIm I don't have enough knowledge to understand at the moment why would static build be broken, but please continue with any changes you think are neccessary to get cmake working and you can explain it to me during the review. Later we can look into fixing Meson -> I am also not an expert on Meson, but there are collaborators we can ask for help if needed.

[SpaceIm]

I've merged master into this branch with some modifications:

• rename EDLIB_DLL_EXPORT to EDLIB_BUILD (this PR also hides symbols with GNU compilers, so it's not only for DLL on Windows).
• remove EDLIB_API in front of typedef (doesn't make sense).
• define EDLIB_API with the original logic of this PR (if EDLIB_SHARED is not defined, EDLIB_API is empty. For static build EDLIB_API must not be __declspec(dllexport) or __declspec(dllimport))
• from my branch: remove EDLIB_BUILD_SHARED in CMakeLists, since it is now defined in the compilation unit prior to include header.

[SpaceIm]

To hide all symbols on GNU compilers with meson, seems that gnu_symbol_visibility: 'inlineshidden' should do the job: https://mesonbuild.com/Reference-manual.html#executable

[Martinsos]

@SpaceIm I see you have done some changes, is that all or are you still adding smth?

I don't completely understand this change: https://github.com/Martinsos/edlib/pull/167/commits/31194a1ce57ddee84944abca54d46e8d954f2616 -> I am guessing it sets correct CFlags for the pkg-config?

Is it correct that besides fixing Meson to work same as CMake and fixing Appveyor to run/test CMake, this is it?

[SpaceIm]

Yes, https://github.com/Martinsos/edlib/commit/31194a1ce57ddee84944abca54d46e8d954f2616 implements a robust method (I hope) to inject interface definitions of edlib (ie EDLIB_SHARED if shared lib) into generated pkg-config file.

And yes, remains modifications of meson.build to work same as CMakeLists.txt:

• manage shared lib with these modifications (hide all symbols on GNU compilers + define EDLIB_SHARED at build and consume time)
• update creation of pkg-config file to add -DEDLIB_SHARED in Cflags if shared (+ check that -<major> suffix is added to generated file name if you still want this behaviour)
• check that meson creates the same .so/.dylib shared libs with VERSION and SOVERSION suffixes, than CMake.

[Martinsos]

@SpaceIm ok, great, thanks for the summary!

Regarding Meson stuff -> I can isolate that into separate issue and I or smb else can handle that later then.

Only thing regarding CMake that is left then is that it does not run in Appveyor currently. Is this something you would like to handle as part of this PR, or should I merge this PR with appveyor breaking and then I can look into adding CMake to appveyor later?

@SoapZA , would you mind taking a quick look at the changes? Soap helps me a lot with the CMake/Meson stuff in Edlib and knows more about this stuff than me so I always try to get his approval also, if he has the time.

[Martinsos]

I took a little bit of time to play with Meson and got it working on Windows (appveyor)! So let's make that also part of this PR. What I did:

1. Made meson correctly build the shared library (I think).
2. Ensured that name of the generated pkg-config file is edlib-{MAJOR}.pc, so that should be now consistent with cmake.
3. Ensured that shared libraries use soversion equal to {MAJOR}.

We should still probably do one more check of how are all the things named in cmake vs meson and ensure it is all the same.

@SoapZA could you please take a look at my changes in meson.build, in this PR? I am sure some things can be done better and would love to hear from you regarding that, and of course maybe I did smth very wrong.

[Martinsos]

All right, let's bring this PR to its (happy) end!

A lot of stuff was done, thanks a lot @SpaceIm @madebr and @SoapZA .

From what I can identify, what is left to do is:

1. Make sure artifacts (shared and static libs, pkg config files) have the same names and versions both in Meson and CMake.
2. Take EDLIB_BUILD out of .cpp file and provide it at build time.
3. Take care of https://github.com/Martinsos/edlib/pull/167/files#r509570843 .

I can take care of (1), and I can take care of (2) regarding Meson. @SpaceIm , could you take care of (2) regarding CMake (you can also do Meson if you like), and also of (3)? Then we all give final approvals and we can merge.

[SoapZA]

LGTM, consider squashing some commits to denoise the history

[Martinsos]

Thanks all again, I took care of the remaining issues, so that should be it! I will wait for CI to pass and then merge (@SoapZA normally I squash and rebase, but due to one merge already happening here and due to having so many commits from different authors, I think I will just merge this one). Yay we did it :D!