Allow Ginkgo to Use the --prefix Option
#1534
Conversation
556a62e
to
206a493
Compare
|
@MarcelKoch can you maybe list the behavioral changes in the pkg-config flag generation so we can check them against the code? |
3ecbeba
to
1b71585
Compare
bb25a80
to
95a8e2a
Compare
There was a problem hiding this comment.
LGTM! Honestly I believe we are doing too much in our pkg-config generation, ideally it should only need
Libs: -lginkgo(d) -lginkgo_* <mpi link flags>
Cflags: -I${includedir}
I am not even sure if the MPI headers (and libraries) should be listed here, as they would need to be available for applications anyways. I would probably be happiest if we don't need to support static linking via pkg-config, because then we can also ignore all transitive dependencies and in a future release (see #715) require users to only link against libginkgo(d).so
95a8e2a
to
cd741d2
Compare
INSTALL.md
Outdated
| For example, to build everything (in debug mode), use: | ||
|
|
||
| ```cmake | ||
| cmake -G "Unix Makefiles" -H. -BDebug -DCMAKE_BUILD_TYPE=Debug -DGINKGO_DEVEL_TOOLS=ON \ | ||
| cmake .. -BDebug -DCMAKE_BUILD_TYPE=Debug -DGINKGO_DEVEL_TOOLS=ON \ |
There was a problem hiding this comment.
can be simplified a bit
| cmake .. -BDebug -DCMAKE_BUILD_TYPE=Debug -DGINKGO_DEVEL_TOOLS=ON \ | |
| cmake .. -BDebug -DCMAKE_BUILD_TYPE=Debug \ |
INSTALL.md
Outdated
| For example, to build everything (in debug mode), use: | ||
|
|
||
| ```cmake | ||
| cmake -G "Unix Makefiles" -H. -BDebug -DCMAKE_BUILD_TYPE=Debug -DGINKGO_DEVEL_TOOLS=ON \ | ||
| cmake .. -BDebug -DCMAKE_BUILD_TYPE=Debug -DGINKGO_DEVEL_TOOLS=ON \ | ||
| -DGINKGO_BUILD_TESTS=ON -DGINKGO_BUILD_REFERENCE=ON -DGINKGO_BUILD_OMP=ON \ | ||
| -DGINKGO_BUILD_CUDA=ON -DGINKGO_BUILD_HIP=ON |
There was a problem hiding this comment.
| -DGINKGO_BUILD_CUDA=ON -DGINKGO_BUILD_HIP=ON | |
| -DGINKGO_BUILD_CUDA=ON -DGINKGO_BUILD_HIP=ON -DGINKGO_BUILD_MPI=on |
test/test_pkgconfig/CMakeLists.txt
Outdated
| @@ -2,7 +2,7 @@ cmake_minimum_required(VERSION 3.16) | |||
| project(GinkgoExportBuildWithPkgConfigTest LANGUAGES CXX) | |||
|
|
|||
| find_package(PkgConfig REQUIRED) | |||
| pkg_check_modules(GINKGO REQUIRED IMPORTED_TARGET ginkgo) | |||
| pkg_check_modules(GINKGO REQUIRED IMPORTED_TARGET "ginkgo_${CMAKE_BUILD_TYPE}") | |||
There was a problem hiding this comment.
Is there no imported target with just ginkgo now ?
There was a problem hiding this comment.
The IMPORTED_TARGET is just an option for PkgConfig, it has nothing to do with the package name. But I missed that we also installed the ginkgo.pc. I think that should be possible to fix.
141acd7
to
9ca0da6
Compare
9ca0da6
to
2c6836b
Compare
|
This PR allows Ginkgo to use the
--prefixoption ofcmake --install. Before, the path set by--prefixwould just be ignored and theCMAKE_INSTALL_PREFIXset at configure time would be used.In principle, this is a minor change, except for the PKG config file. Getting this to work is quite cumbersome, but now it should work as expected.
Changes to the pkg-config file:
prefixnow uses the path specified by--prefix(if present)libdirandincludedirare now relative toprefix-Land-Ipaths for ginkgo also use paths relative toprefix