Skip to content

Add basic config install#247

Closed
xsacha wants to merge 1 commit intoshibatch:masterfrom
xsacha:master
Closed

Add basic config install#247
xsacha wants to merge 1 commit intoshibatch:masterfrom
xsacha:master

Conversation

@xsacha
Copy link
Copy Markdown

@xsacha xsacha commented Mar 5, 2019

Will allow you to find with find_package(sleef) and use with target_link_libraries(${TARGET_NAME} sleef::sleef).

Only basic right now as I am unfamilar with the project and not sure if there are any special cmake defines that need to be passed through.

@shibatch
Copy link
Copy Markdown
Owner

shibatch commented Mar 6, 2019

@xsacha Thank you for submitting a patch.
Unfortunately, the patched source code cannot be built.
Please check the output from CI server and fix the problem.

https://travis-ci.org/shibatch/sleef/jobs/501824573

@xsacha
Copy link
Copy Markdown
Author

xsacha commented Mar 6, 2019 •
edited
Loading

Hey, sorry, I am confused by this option:
option(BUILD_LIBM "libsleef will be built." ON)
I don't think the text is right for this. It seems to always build this library regardless.

Specific isue with build is a CMake issue I'll have to workaround: https://cmake.org/Bug/view.php?id=14444

@shibatch
Copy link
Copy Markdown
Owner

shibatch commented Mar 7, 2019

Why do you need to install ${TARGET_LIBSLEEF}, ${TARGET_LIBSLEEFGNUABI} and ${TARGET_LIBDFT} at the end of the top CMakeLists.txt?

I thought what you need to additionally install is *.cmake.

@xsacha
Copy link
Copy Markdown
Author

xsacha commented Mar 11, 2019

Yeah, that's how it works. I give it the TARGET and it brings in all associated libraries, dependencies, includes (unless you did them manually somewhere like the find_package).

That first attempt just didn't work because of a bug/design in that specific CMake yet it worked fine in mine (latest).

This new version should work fine in all CMake.

@xsacha xsacha force-pushed the master branch 2 times, most recently from de4306f to af5668c Compare March 11, 2019 05:06
@xsacha @ruslo
Add simple config (shibatch#1)
acc49b8 
* Add simple config

* Add config dependencies (OpenMP isn't used yet in the latest release)
@xsacha xsacha force-pushed the master branch 3 times, most recently from de4306f to acc49b8 Compare March 13, 2019 03:24
@shibatch
Copy link
Copy Markdown
Owner

shibatch commented Apr 9, 2020

@xsacha I know I am late, but I am now looking into this PR.
Could you tell me how those atmark-surrounded variables work?

@xsacha
Copy link
Copy Markdown
Author

xsacha commented Apr 10, 2020

@shibatch when the file is configured, as in configure_package_config_file, the @variables@ are replaced by the CMAKE variables with the same name.

@shibatch
Copy link
Copy Markdown
Owner

shibatch commented Apr 10, 2020

Looks good, but I think we need some kind of a tester for making sure that this functionality is not broken.

@shibatch shibatch requested a review from fpetrogalli April 10, 2020 02:18
@shibatch
Copy link
Copy Markdown
Owner

shibatch commented Apr 10, 2020

@fpetrogalli How do you think? If you are happy with this patch, I will merge this and then add a tester.

@friendlyanon friendlyanon mentioned this pull request Feb 19, 2021
Merged
@friendlyanon
Copy link
Copy Markdown
Contributor

friendlyanon commented Apr 1, 2021

@shibatch #412 superseded this PR

@blapie
Copy link
Copy Markdown
Contributor

blapie commented Dec 7, 2023

I'm closing this PR for the reason pointed by @friendlyanon.

@blapie blapie closed this Dec 7, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@fpetrogalli fpetrogalli Awaiting requested review from fpetrogalli

Assignees

No one assigned

Labels

cmake enhancement

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

4 participants

@xsacha @shibatch @friendlyanon @blapie