Fix use of the cmake package

[niclasr]

Fix use of the cmake package and its imported targets. Make sure that the include dir comes with the cmake targets Put everything under md4cConfig so that the md4c-html can see md4c. Use md4c namespace so that the targets become md4c::md4c and md4c::md4c-html following cmake standards for imported targets.

Since a prefix has been added to the targets maybe they should be documented in the readme.

[codecov[bot]]

Codecov Report

Merging #146 (43c5436) into master (aa63198) will not change coverage. The diff coverage is n/a.

@@           Coverage Diff           @@
##           master     #146   +/-   ##
=======================================
  Coverage   94.45%   94.45%           
=======================================
  Files           3        3           
  Lines        3066     3066           
=======================================
  Hits         2896     2896           
  Misses        170      170           

---

Continue to review full report at Codecov.

Legend - Click here to learn more Δ = absolute <relative> (impact), ø = not affected, ? = missing data Powered by Codecov. Last update aa63198...43c5436. Read the comment docs.

[mity]

First of all, thank you for the work on it.

I never encountered the CMake namespaces yet, so forgive me asking few (potentially silly) questions.

• Actually, the fact alone I never encountered them so far makes me wonder how much their use is just a recommended practice by CMake authors from academic point of view; or how much widespread/standard it really is in a real-world use. Generally, I tend to be pragmatic and take the way of least surprise in such matters.

• Just to be sure, the change affects only CMake, more specifically how 3rd-party CMakeLists.txt files specify their targets to link against the lib and it affects "only" the build of CMake-based 3rd party applications, right?

  I want to be especially certain there are no new library files built/installed and no changes in paths/names of the library binaries. MD4C is used already in many linux distros and update of it must not break the binary compatibility of already built/shipped applications.

• Why is there add_library(md4c::md4c ALIAS md4c) but no analogical line for md4c-html? Or put differently, where exactly is the md4c-html target made to live in the namespace?

I don't know what you think of the namespace addition since it might break backward compatibility.

I'm C guy more than CMake guy and I never tried/needed to use CMake to its full potential. So right now I'm trying to understand how big the breakage really is (or whether there is any).

If I understand #145 correctly the md4c-html target has been completely broken and unusable, right? (I likely never needed to use that from another CMakeLists.txt except for md2html utility). If so, there cannot be any compatibility issue as it had to be broken from the very start when that lib was separated from md2html for better reuse.

And if the alias md4c for md4c::md4c allows to still refer to it with just md4c, there should be no compatibility issue introduced either, right?

[niclasr]

Just to be sure, the change affects only CMake, more specifically how 3rd-party CMakeLists.txt files specify their targets to link against the lib and it affects "only" the build of CMake-based 3rd party applications, right?

I want to be especially certain there are no new library files built/installed and no changes in paths/names of the library binaries. MD4C is used already in many linux distros and update of it must not break the binary compatibility of already built/shipped applications.

Yes it only affects 3rdparty CMake builds that depend on md4c via find_package(md4c) command, no changes to the library names and the pkg-config files.

If I understand #145 correctly the md4c-html target has been completely broken and unusable, right? (I likely never needed to use that from another CMakeLists.txt except for md2html utility). If so, there cannot be any compatibility issue as it had to be broken from the very start when that lib was separated from md2html for better reuse.

Yes due to the difference between the directory name and the config name, so moving the target md4c-html to md4cConfig is perfectly safe.

Why is there add_library(md4c::md4c ALIAS md4c) but no analogical line for md4c-html? Or put differently, where exactly is the md4c-html target made to live in the namespace?

I thought this was needed for md4-html to be dependent on md4c::md4c in the generated md4cConfig. I just tested without it and made md4c-hml depend on md4c as usual, the generated file is the same so the line is useless, I will remove it. The targets are made to live under the md4c:: namespace by the NAMESPACE md4c:: in the install command on the last line.

Actually, the fact alone I never encountered them so far makes me wonder how much their use is just a recommended practice by CMake authors from academic point of view; or how much widespread/standard it really is in a real-world use. Generally, I tend to be pragmatic and take the way of least surprise in such matters.

And if the alias md4c for md4c::md4c allows to still refer to it with just md4c, there should be no compatibility issue introduced either, right?

The alias command used make md4c::md4c and alias form md4c and as mentioned above it is not needed. Due to the namespace the exported target name will change and that breaks CMake builds that use the md4c target. The Cmake authors write this about namespaces in their export guide

The NAMESPACE option will prepend MathFunctions:: to the target names as they are written to the export file. This convention of double-colons gives CMake a hint that the name is an IMPORTED target when it is used by downstream projects. This way, CMake can issue a diagnostic message if the package providing it was not found.

Well this might not be enough reason for you to add a namespace that breaks backward compatibility with CMake builds that depends on md4c via find_package(md4c) . If you want backward compatibility for 3rd party CMake builds I can just remove the NAMESPACE md4c:: part and it works just like before, the choice is up to you.

[mity]

I just tested without it and made md4c-hml depend on md4c as usual, the generated file is the same so the line is useless, I will remove it.

Please do, and I'll merge.

If you want backward compatibility for 3rd party CMake builds I can just remove the NAMESPACE md4c:: part and it works just like before, the choice is up to you.

If you think that (except the compatibility) it's better to use the namespace, lets keep it as it is, at least for now. If it happens people complain before the next release (probably not anytime soon) we might reconsider.

[mity]

Merged, thank you for working on it.