Skip to content

Added CMake support for library #55

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

Closed
wants to merge 1 commit into from
Closed

Conversation

Romop5
Copy link

@Romop5 Romop5 commented Feb 18, 2018

The purpose of this PR is to integrate CMake support directly to project and preserve the current Makefile-correctness at the same time.
The reason for adding CMakeLists.txt is to be able to integrate this library in other projects using add_subdirectory() feature in CMake.
Also, CMakeLists.txt for this library is desired as it provides include & link settings for library which would one have to add manually in his project.

This PR has been tested on both Linux (Fedora, gcc/clang) and Windows (VS).

@lava
Copy link
Owner

lava commented Sep 28, 2018

Hi, I'm sorry for not commenting on this for so long. Long story short, I'm still hesitant to merge CMake support, first of all because I don't want to support the CMakeLists.txt and second because matplotlib-cpp isn't really working as a "proper" library, since linking against it will cause binaries that depend (maybe indirectly) on libpython otherwise to crash. So adding this PR would feel a bit like setting people up to run into a knife.

However, I'll leave it open so people who actually want the CMake support can easily find it, or if you prefer you could put it into the contrib/ folder instead.

@cdbrkfxrpt
Copy link
Contributor

I'm in favor of merging this. It makes matplotlibcpp a CMake INTERFACE (read: header only) library and thus changes absolutely nothing for people who just use the header directly but makes it easier for people who use CMake, as all we then have to do is clone the repo to our thirdparty directory (or add submodule or whatever) and then

add_subdirectory (${PROJECT_SOURCE_DIR}/thirdparty/matplotlib-cpp}
# ...
target_link_libraries (my_target PRIVATE matplotlib-cpp ...)

@acxz
Copy link
Contributor

acxz commented Jan 25, 2020

@lava Are you still willing to consider merging this feature? As more and more projects use this header only library (which is amazing btw!), it makes using it in a CMake project easier and prevents code duplication. I would also like to package this for ArchLinux and having a CMake install targets makes it so much easier.

@lava lava mentioned this pull request Mar 25, 2021
@lava
Copy link
Owner

lava commented Mar 25, 2021

As said in #249, the formerly simple Makefile has become more and more complicated, so I think we're at the point where switching to cmake makes sense. (I still think it's not a good idea to use this as library in other projects, but clearly that's not stopping people from doing it anyways)

I've opted for merging the other PR instead since it seemed a bit more modern and feature-complete, but thanks again for providing the original cmake support and enabling interested users to make use of it with your PR!

@lava lava closed this Mar 25, 2021
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