Skip to content

Include cmake files in install#344

Merged
jbeder merged 1 commit into
jbeder:masterfrom
paulnovo:cmake-install
Mar 28, 2016
Merged

Include cmake files in install#344
jbeder merged 1 commit into
jbeder:masterfrom
paulnovo:cmake-install

Conversation

@paulnovo

Copy link
Copy Markdown
Contributor

This adds yaml-cpp-config.cmake, yaml-cpp-config-version.cmake, and
yaml-cpp-targets.cmake to the cmake install. As a result, cmake's
find_package can easily find yaml-cpp for software that depends on
yaml-cpp.

Closes #336 #127

Comment thread CMakeLists.txt Outdated

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

What's this OPENSURGSIM_CMAKE_DIR?

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Looks like some of this was taken from this project. Definitely not standard. Needs to be edited out.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Oops, yes this is a mistake.

@theNerd247

Copy link
Copy Markdown

I'm getting:

CMake Error: install(EXPORT "yaml-cpp-targets") given absolute DESTINATION "/usr/local/lib/cmake/yaml-cpp" but the export references an installation of target "yaml-cpp" which has relative DESTINATION "lib".

When running cmake on a clean build.

Would my branch perform the same as this PR? The edits I've made are much simpler.

@paulnovo

paulnovo commented Dec 1, 2015

Copy link
Copy Markdown
Contributor Author

@theNerd247 Does the latest version fix your issue?

@jbeder

jbeder commented Jan 10, 2016

Copy link
Copy Markdown
Owner

I'm getting the same error as @theNerd247 above.

@drlight-code

Copy link
Copy Markdown

For me it builds and installs fine, cmake 3.4.1 here.

@jbeder

jbeder commented Jan 31, 2016

Copy link
Copy Markdown
Owner

Interesting, I'm getting the error with cmake 3.1.x, but it succeeds when upgrading to 3.4.x.

Paul, any idea why?

@paulnovo

Copy link
Copy Markdown
Contributor Author

I just tried it with cmake 3.0, and I can now reproduce the error. I'll
take a look.

On Sun, Jan 31, 2016, at 01:02 PM, Jesse Beder wrote:

Interesting, I'm getting the error with cmake 3.1.x, but it succeeds
when upgrading to 3.4.x.

Paul, any idea why?

— Reply to this email directly or view it on GitHub[1].

Links:

  1. Include cmake files in install #344 (comment)

@drlight-code

Copy link
Copy Markdown

Since the maintainer is not notified if someone only pushes to a PR branch, I post a reminder comment, the commit message suggests Paul fixed it.

@drlight-code

Copy link
Copy Markdown

Please test this again and merge eventually.

@jbeder

jbeder commented Mar 26, 2016

Copy link
Copy Markdown
Owner

Looks good, sorry for the delay.

Please squash these commits and rebase against HEAD.

@jbeder jbeder mentioned this pull request Mar 26, 2016
This adds yaml-cpp-config.cmake, yaml-cpp-config-version.cmake, and
yaml-cpp-targets.cmake to the cmake install. As a result, cmake's
find_package can easily find yaml-cpp for software that depends on
yaml-cpp.

Add code to install cmake files to $CMAKE_INSTALL_PREFIX/CMake on
Windows, which is the de-facto standard.

Closes jbeder#336 jbeder#127
@paulnovo

Copy link
Copy Markdown
Contributor Author

@jbeder Ok, I think it is all set.

@ishitatsuyuki

Copy link
Copy Markdown
Contributor

Just a tip: use GNUInstallDirs for pretty variables of the install destination.

@jbeder jbeder merged commit 500db60 into jbeder:master Mar 28, 2016
@ishitatsuyuki

Copy link
Copy Markdown
Contributor

This contains some unneeded changes. Will create a cleanup PR then.

@DinoStray

Copy link
Copy Markdown

How to use it?
Any YAMLCPP_INCLUDE_DIR or YAMLCPP_LIBRARY variable?

@atar-axis

Copy link
Copy Markdown

@DinoStray Don't use it at all, CMake variables are deprecated... see #661

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.

8 participants