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

Generate and install cmake-config file #368

Merged
merged 1 commit into from Dec 3, 2020

Conversation

NeroBurner
Copy link
Contributor

Add full cmake support. The project can either be used with
add_subdirectory or be installed into the system (or some other
directory) and be found with find_package(cmark). In both cases the
cmake target cmark::cmark and/or cmark::cmark_static is all that
is needed to be linked.

Previously the cmarkConfig.cmake file was generated, but not installed.
As additional bonus of generation by cmake we get a generated
cmake-config-version.cmake file for find_package() to search for
the same major version.

The generated config file is position independent, allowing the
installed directory to be copied or moved and still work.

The following four files are generated and installed:

  • lib/cmake/cmark/cmark-config.cmake
  • lib/cmake/cmark/cmark-config-version.cmake
  • lib/cmake/cmark/cmark-targets.cmake
  • lib/cmake/cmark/cmark-targets-release.cmake

Add full cmake support. The project can either be used with
`add_subdirectory` or be installed into the system (or some other
directory) and be found with `find_package(cmark)`. In both cases the
cmake target `cmark::cmark` and/or `cmark::cmark_static` is all that
is needed to be linked.

Previously the cmarkConfig.cmake file was generated, but not installed.
As additional bonus of generation by cmake we get a generated
`cmake-config-version.cmake` file for `find_package()` to search for
the same major version.

The generated config file is position independent, allowing the
installed directory to be copied or moved and still work.

The following four files are generated and installed:
- lib/cmake/cmark/cmark-config.cmake
- lib/cmake/cmark/cmark-config-version.cmake
- lib/cmake/cmark/cmark-targets.cmake
- lib/cmake/cmark/cmark-targets-release.cmake
@jgm
Copy link
Member

jgm commented Nov 26, 2020

Seems like a good idea, bit I'm too much of a novice with cmake to evaluate this.
@nwellnhof do these changes seem okay to you?

@nwellnhof
Copy link
Contributor

I have only rudimentary knowledge of CMake and comment on that.

@NeroBurner
Copy link
Contributor Author

A minimal example to try out with this branch is the following CMakeLists.txt file

cmake_minimum_required(VERSION 3.1)
project(finding_cmark)
find_package(cmark CONFIG REQUIRED)
# target_link_libraries(${PROJECT_NAME} PRIVATE cmark::cmark)

compile the cmark project and install it into a subfolder

cmake -S . -B build -DCMAKE_INSTALL_PREFIX="${PWD}/install"
cmake --build build --target install -j4

then change to the minimal finding_cmark example and pass it the directory to the installed cmark-config.cmake

cmake -S . -B build -Dcmark_DIR=/path/to/cmark/project/install/lib/cmake/cmark/

The cmark_DIR is only necessary for development if you don't want to install into the system paths. If cmark is installed into the search path (like when installed as system package) the cmark_DIR variable is not needed

@NeroBurner
Copy link
Contributor Author

can I do something to help get this merged?

Comment on lines -8 to +10
target_link_libraries(api_test libcmark)
target_link_libraries(api_test cmark)
else()
target_link_libraries(api_test libcmark_static)
target_link_libraries(api_test cmark_static)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why do the library names change? Is that essential? What effect does it have?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The name of the target in CMakeLists.txt dictates the name of the generated target in the cmark-config.cmake. When searching with find_package(cmark) in CMake the expectation is a target like cmark::cmark. The cmark:: part is an indication of an imported target (not compiled by your current project, but imported from 'outside'

Comment on lines -5 to 7
set(LIBRARY "libcmark")
set(STATICLIBRARY "libcmark_static")
set(LIBRARY "cmark")
set(STATICLIBRARY "cmark_static")
set(HEADERS
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ditto - why the name change?

set(PROGRAM "cmark")
set(PROGRAM "cmark_exe")
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

why the name change?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

the cmark_exe never leaves the project, unlike the library target cmark which is exported into cmark-config.cmake

@jgm
Copy link
Member

jgm commented Dec 1, 2020

I'm just reluctant to merge something if I don't understand all the implications. You may have to walk me through some of the details...

@jgm
Copy link
Member

jgm commented Dec 1, 2020

Mainly I want to be very sure that nothing that currently works breaks!

@NeroBurner
Copy link
Contributor Author

Mainly I want to be very sure that nothing that currently works breaks!

Very understandable and very good to hear. What are the things that are important and need to be checked?

@jgm
Copy link
Member

jgm commented Dec 2, 2020

What are the things that are important and need to be checked?

After this change, will make install behave differently (aside from installing the cmake config file for cmark)? Will other things all go to the same spots as before and have the same names?

@NeroBurner
Copy link
Contributor Author

The install target from branch master

cmake --build build --target install # branch master
[ 43%] Built target libcmark
[ 86%] Built target libcmark_static
[ 95%] Built target api_test
[100%] Built target cmark
Install the project...
-- Install configuration: "Release"
-- Up-to-date: /home/nero/repos/plasma-mobile/cmark/_install/bin/cmark
-- Up-to-date: /home/nero/repos/plasma-mobile/cmark/_install/lib/libcmark.so.0.29.0
-- Up-to-date: /home/nero/repos/plasma-mobile/cmark/_install/lib/libcmark.so
-- Up-to-date: /home/nero/repos/plasma-mobile/cmark/_install/lib/libcmark.a
-- Up-to-date: /home/nero/repos/plasma-mobile/cmark/_install/lib/pkgconfig/libcmark.pc
-- Up-to-date: /home/nero/repos/plasma-mobile/cmark/_install/include/cmark.h
-- Up-to-date: /home/nero/repos/plasma-mobile/cmark/_install/include/cmark_export.h
-- Up-to-date: /home/nero/repos/plasma-mobile/cmark/_install/include/cmark_version.h
-- Up-to-date: /home/nero/repos/plasma-mobile/cmark/_install/lib/cmake/cmark.cmake
-- Up-to-date: /home/nero/repos/plasma-mobile/cmark/_install/lib/cmake/cmark-release.cmake
-- Up-to-date: /home/nero/repos/plasma-mobile/cmark/_install/share/man/man1/cmark.1
-- Up-to-date: /home/nero/repos/plasma-mobile/cmark/_install/share/man/man3/cmark.3

in comparison the install target from this new branch cmake_config_target_generation

cmake --build build --target install # branch cmake_config_target_generation
[ 43%] Built target cmark_static
[ 86%] Built target cmark
[ 91%] Built target cmark_exe
[100%] Built target api_test
Install the project...
-- Install configuration: "Release"
-- Up-to-date: /home/nero/repos/plasma-mobile/cmark/_install/bin/cmark
-- Up-to-date: /home/nero/repos/plasma-mobile/cmark/_install/lib/libcmark.so.0.29.0
-- Up-to-date: /home/nero/repos/plasma-mobile/cmark/_install/lib/libcmark.so
-- Up-to-date: /home/nero/repos/plasma-mobile/cmark/_install/lib/libcmark.a
-- Up-to-date: /home/nero/repos/plasma-mobile/cmark/_install/lib/pkgconfig/libcmark.pc
-- Up-to-date: /home/nero/repos/plasma-mobile/cmark/_install/include/cmark.h
-- Up-to-date: /home/nero/repos/plasma-mobile/cmark/_install/include/cmark_export.h
-- Up-to-date: /home/nero/repos/plasma-mobile/cmark/_install/include/cmark_version.h
-- Up-to-date: /home/nero/repos/plasma-mobile/cmark/_install/lib/cmake/cmark/cmark-config.cmake
-- Up-to-date: /home/nero/repos/plasma-mobile/cmark/_install/lib/cmake/cmark/cmark-config-version.cmake
-- Up-to-date: /home/nero/repos/plasma-mobile/cmark/_install/lib/cmake/cmark/cmark-targets.cmake
-- Up-to-date: /home/nero/repos/plasma-mobile/cmark/_install/lib/cmake/cmark/cmark-targets-release.cmake
-- Up-to-date: /home/nero/repos/plasma-mobile/cmark/_install/share/man/man1/cmark.1
-- Up-to-date: /home/nero/repos/plasma-mobile/cmark/_install/share/man/man3/cmark.3

the generated file cmark_export.h has a slight change (but this is only for the build-time)

diff --git a/_install_master/include/cmark_export.h b/_install_cmake/include/cmark_export.h
index 9e5dd5d..3dc3165 100644
--- a/_install_master/include/cmark_export.h
+++ b/_install_cmake/include/cmark_export.h
@@ -7,7 +7,7 @@
 #  define CMARK_NO_EXPORT
 #else
 #  ifndef CMARK_EXPORT
-#    ifdef libcmark_EXPORTS
+#    ifdef cmark_EXPORTS
         /* We are building this library */
 #      define CMARK_EXPORT __attribute__((visibility("default")))
 #    else

Other files did not change. Only the deleted cmark-release.cmake and cmark.cmake are shown with git diff --no-index _install_master _install_cmake. Those are expected changes.

The so-names and pkgconfig/libcmark.pc contents are unchanged

@jgm
Copy link
Member

jgm commented Dec 3, 2020

OK, I'm satisfied. Thank you.

@jgm jgm merged commit c8d5be9 into commonmark:master Dec 3, 2020
@NeroBurner NeroBurner deleted the cmake_config_target_generation branch December 4, 2020 14:12
kdesysadmin pushed a commit to KDE/neochat that referenced this pull request Dec 6, 2020
Tell Findcmark.cmake to first search for a `cmark-config.cmake` file
introduced with PR commonmark/cmark#368

If no config file can be found (which provides `cmark::cmark` target use
pkg_config as fallback.
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.

None yet

3 participants