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 modernization #84

Merged
merged 4 commits into from
Mar 17, 2023
Merged

CMake modernization #84

merged 4 commits into from
Mar 17, 2023

Conversation

seanharmer
Copy link
Contributor

These changes makes it much easier to use lunasvg in downstream projects either by hand or via CMake's FetchContent.

To use targets and properties so that lunasvg can be more
easily added to external projects using cmake's FetchContent.
@sammycage
Copy link
Owner

Wonderful work... I'll take a closer look. <3

@seanharmer
Copy link
Contributor Author

Have you had a chance to take a look at this please?

@fdwr
Copy link
Contributor

fdwr commented May 19, 2022

@seanharmer

  • Will include(GNUInstallDirs) bork the CMakeLists.txt for me on Windows?
  • Where is lunasvg_export.h defined? (or lunasvgexport.h per existing naming conventions) I don't see it in this PR.

@seanharmer
Copy link
Contributor Author

  • Will include(GNUInstallDirs) bork the CMakeLists.txt for me on Windows?

I think it's fine. It just helps put the build artifacts under install dir/bin install dir/include etc. It worked for me on Windows anyway.

  • Where is lunasvg_export.h defined? (or lunasvgexport.h per existing naming conventions) I don't see it in this PR.

Ah right. This header file is generated into the build dir when you run cmake. The line that generates it is:

include(GenerateExportHeader)
generate_export_header(lunasvg)

It just saves us hand crafting a suitable export header for shared/static libs. The full documentation is at:

https://cmake.org/cmake/help/latest/module/GenerateExportHeader.html

Hope this helps! :)

@dg0yt dg0yt mentioned this pull request Mar 16, 2023
6 tasks
@Pospelove
Copy link

Good job. This is an important change. Hope it lands at some point :)

@sammycage sammycage merged commit b7e72fb into sammycage:master Mar 17, 2023
@sammycage
Copy link
Owner

Thank you, @seanharmer. I apologize for the delay in responding.

sammycage added a commit that referenced this pull request May 26, 2023
This reverts commit b7e72fb, reversing
changes made to 4c16abf.
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.

4 participants