-
Notifications
You must be signed in to change notification settings - Fork 86
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
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.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
-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.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is there no imported target with just ginkgo
now ?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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
--prefix
option ofcmake --install
. Before, the path set by--prefix
would just be ignored and theCMAKE_INSTALL_PREFIX
set 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:
prefix
now uses the path specified by--prefix
(if present)libdir
andincludedir
are now relative toprefix
-L
and-I
paths for ginkgo also use paths relative toprefix