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 fixes #137

Merged
merged 3 commits into from
Jul 15, 2016
Merged

CMake fixes #137

merged 3 commits into from
Jul 15, 2016

Conversation

foonathan
Copy link
Contributor

Just two fixes to use the library better as CMake subproject (add_subdirectory()).

  1. Export the targets for installation. This will create a file generating an IMPORT target, required for e.g. find_package() and allows linking the cmark targets to your own targets if your own targets are installed.
  2. Remove a developer warning by suppressing the policy (see comments).

@@ -29,3 +29,6 @@ if(NOT CMAKE_BUILD_TYPE)
set(CMAKE_BUILD_TYPE "Release" CACHE STRING
"Choose the type of build, options are: Debug Profile Release Asan Ubsan." FORCE)
endif(NOT CMAKE_BUILD_TYPE)

# export targets
install(EXPORT cmark DESTINATION lib/cmake)
Copy link
Member

Choose a reason for hiding this comment

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

Is this really supposed to be lib/cmake?

Copy link
Member

Choose a reason for hiding this comment

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

Also note the comment at line 49 of src/CMakeLists.txt.

@foonathan
Copy link
Contributor Author

CMake has a couple of places where it searches for config files generated by the install(EXPORT) command. /lib/cmake is one of them, useful if your project doesn't add its own subfolder for the libraries (like this here does).

I can change that to ${LIB_INSTALL_DIR}/cmake, didn't see the comment.

@foonathan
Copy link
Contributor Author

When is this PR going to be merged?

@jgm
Copy link
Member

jgm commented Jul 14, 2016

I'm just worried about assuming that the build machine will always have a $LIB_INSTALL_DIR/cmake directory.

Is that universally true, even on Windows e.g.?

@foonathan
Copy link
Contributor Author

The common CMake convention is that you install and export libraries under lib/project. But to suppport installing to lib directly you can also export under lib/cmake.

And the documentation for find_package() lists <prefix>/lib/cmake. It doesn't assume that the project has such a directory, it will be created when needed (afaik).

@jgm jgm merged commit 03b5d60 into commonmark:master Jul 15, 2016
@jgm
Copy link
Member

jgm commented Jul 15, 2016

OK, I've merged now!

@foonathan
Copy link
Contributor Author

Thanks! If you run into any CMake issues related to that, I'll be happy to help.

CyberShadow pushed a commit to CyberShadow/cmark that referenced this pull request Apr 1, 2021
…red (commonmark#137)

fdopen, strdup and others are not declared by glibc header files
unless _DEFAULT_SOURCE is defined.

Signed-off-by: Keith Packard <keithp@keithp.com>
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.

2 participants