mithun218 / GiNaCDE

GiNaC Differential Equation Solver
MIT License
9 stars 1 forks source link

Support and advertise usage in downstream build systems #8

Closed peanutfun closed 2 years ago

peanutfun commented 3 years ago

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 commented 3 years ago

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

mithun218 commented 3 years ago

done!

peanutfun commented 2 years ago

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 commented 2 years ago

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

mithun218 commented 2 years ago

I have implemented it here.

peanutfun commented 2 years ago

Great! 👍