Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

CMake improvements #261

Open
wants to merge 2 commits into
base: master
Choose a base branch
from
Open

Conversation

Tachi107
Copy link

@Tachi107 Tachi107 commented Aug 9, 2024

Hi! First of all, great work! In my opinion, md4c is the most interesting Markdown library of all :)

With this patch set I've reworked a bit the CMake files, to make them a bit more "modern" (what "modern" really means is up to the reader). I've also fixed some actual (small) issues.

I've split this in two separate commits since they are two separate kind of fixes, but if you prefer to have a single commit instead let me know! GitHub's Squash feature kinda sucks and tends to drop commit messages.

Here's the description of the individual changes:

build: CMake improvements

This patch improves and modernizes the CMake build scripts in multiple
ways, while also fixing some bugs:

  • Requirements like include directories, compile definitions, and
    libraries are now assigned to targets explicitly. This has the
    benefits of propagating requirements transitively, which is especially
    helpful when using md4c as a subproject or via the CMake Config files.
  • The new project() syntax is used, meaning that manually splitting the
    version number in different MD_VERSION_* variables is no longer
    needed. I also made use of the HOMEPAGE_URL option, which requires
    CMake 3.12. This can be reverted in case compatibility with older
    CMake versions is desired.
  • Changed ifs to use variables instead of explicitly evaluating them
    with ${} syntax.
  • Dropped the MD4C_USE_UTF8 compile definition from CMake code, so
    that users can more easily override the default. UTF-8 is used by
    default automatically if no relevant definition is found since commit
    64bf660.
  • Changed DEBUG to only be defined when compiling md4c in debug mode
    via a generator expression (documented in
    https://cmake.org/cmake/help/latest/manual/cmake-buildsystem.7.html#build-configurations).
  • Different include directory paths are when building and when
    installing the md4c and md4c-html targets, thanks to a generator
    expression (documented in
    https://cmake.org/cmake/help/latest/command/target_include_directories.html).
    This makes the INCLUDES DESTINATION install() option redundant,
    and has been removed.

build: handle absolute include and lib dirs in pc file

With this patch, the pkg-config files are now correctly generated when
the include and/or lib directories are absolute paths outside of
CMAKE_INSTALL_PREFIX, which (if I recall correctly) is commonly the case
on distributions like Nix.

For further information, see
https://github.com/jtojnar/cmake-snips#assuming-cmake_install_dir-is-relative-path.

This patch improves and modernizes the CMake build scripts in multiple
ways, while also fixing some bugs:

- Requirements like include directories, compile definitions, and
  libraries are now assigned to targets explicitly. This has the
  benefits of propagating requirements transitively, which is especially
  helpful when using md4c as a subproject or via the CMake Config files.
- The new project() syntax is used, meaning that manually splitting the
  version number in different MD_VERSION_* variables is no longer
  needed. I also made use of the HOMEPAGE_URL option, which requires
  CMake 3.12. This can be reverted in case compatibility with older
  CMake versions is desired.
- Changed ifs to use variables instead of explicitly evaluating them
  with `${}` syntax.
- Dropped the `MD4C_USE_UTF8` compile definition from CMake code, so
  that users can more easily override the default. UTF-8 is used by
  default automatically if no relevant definition is found since commit
  64bf660.
- Changed `DEBUG` to only be defined when compiling md4c in debug mode
  via a generator expression (documented in
  <https://cmake.org/cmake/help/latest/manual/cmake-buildsystem.7.html#build-configurations>).
- Different include directory paths are when building and when
  installing the md4c and md4c-html targets, thanks to a generator
  expression (documented in
  <https://cmake.org/cmake/help/latest/command/target_include_directories.html>).
  This makes the `INCLUDES DESTINATION` `install()` option redundant,
  and has been removed.
With this patch, the pkg-config files are now correctly generated when
the include and/or lib directories are absolute paths outside of
CMAKE_INSTALL_PREFIX, which (if I recall correctly) is commonly the case
on distributions like Nix.

For further information, see
<https://github.com/jtojnar/cmake-snips#assuming-cmake_install_dir-is-relative-path>.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant