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

Support and advertise usage in downstream build systems #8

Closed
peanutfun opened this issue Nov 10, 2021 · 6 comments
Closed

Support and advertise usage in downstream build systems #8

peanutfun opened this issue Nov 10, 2021 · 6 comments

Comments

@peanutfun
Copy link

Summary

The README.md advertises to install GiNaCDE in the default installation directory and then link it "by hand" when compiling new executables. However, as CMake project, GiNaCDE should support being used in downstream CMake projects via simple find_package(GiNaCDE). To do so, it is best practice for a CMake project to install its own package configuration. See, for example, the excellent blog post "It's Time To Do CMake Right". Currently, GiNaCDE installs only a pkg-config file. This is fine as it enables downstream modules to at least get the most important information on the package. However, the README.md currently contains no instructions on how to do so.

Proposal 1 is easy to do and a simple addition to the current README.md. The more complete approach would be Proposal 2, but I realize that this might only be available to advanced users of CMake. However, I want to advertise it nonetheless, because it is the best way of ensuring your project can be used by others.

Proposal 1: Show how to use GiNaCDE downstream

If we create a new CMake project that uses GiNaCDE, we need to link the GiNaCDE library against the project executables. GiNaCDE provides a pkg-config configuration. So we currently need to do the following:

find_package(PkgConfig)
pkg_search_module(GiNaCDE REQUIRED IMPORTED_TARGET GiNaCDE>=1.0)
add_executable(my_example my_example.cpp)
target_link_libraries(my_example PRIVATE GiNaCDE)

This should be reflected in the README.md, as the command in README.md#Execution only works if GiNaCDE is installed into the default installation directory (or any other default compiler lookup path). Importantly, pkg-config has its own lookup paths that might have to be adapted depending on where GiNaCDE is installed.

Notice also that the README.md currently advertises to #include <GiNaCDE/GiNaCDE.h>, but the include directory set in GiNaCDE.pc.in is @CMAKE_INSTALL_PREFIX@/include/GiNaCDE. This means that when using the pkg-config configuration, one needs to #include <GiNaCDE.h>.

Proposal 2: Make GiNaCDE install a proper CMake configuration

Ideally, we would like to call find_package in a downstream project, like so:

find_package(GiNaCDE 1.0 REQUIRED)
add_executable(my_example my_example.cpp)
target_link_libraries(my_example PRIVATE GiNaCDE)

This requires the following to be installed alongside the headers and libraries:

Related issues

openjournals/joss-reviews#3885

@mithun218
Copy link
Owner

@peanutfun Thank you for the great suggestions! I will make all of the changes you suggested!

@mithun218
Copy link
Owner

done!

@peanutfun
Copy link
Author

Thank you! You now mention the issue with the #include path of GiNaCDE in the README.md, but I think this is a bit confusing. I suggest you change the include path in ginacde.pc.in to @CMAKE_INSTALL_PREFIX@/include/, so that #include <GiNaCDE/GiNaCDE.h> is always correct. Then the source files do not need to change depending on the way GiNaCDE is installed or included into a downstream project.

@mithun218
Copy link
Owner

@peanutfun thanks for your great suggestion. I will implement it in a new PR.

@mithun218
Copy link
Owner

I have implemented it here.

@peanutfun
Copy link
Author

Great! 👍

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

No branches or pull requests

2 participants