boostorg / boost_install

8 stars 30 forks source link

CMake configuration file missing dependency to MPI::MPI_C #17

Closed sdebionne closed 4 years ago

sdebionne commented 4 years ago

The CMake configuration files generated at installation with boostorg/boost_install have an imported target that is missing a dependency to the underlying MPI library. The symptoms are an invalid linking order and the associated undefined symbols errors.

@pdimov, what would be the right way to have something like find_dependency(MPI) and target_link_libraries(Boost::mpi PUBLIC MPI::MPI_C) added to the config file? Shall we add a test for MPI to the boost_install test suite?

pdimov commented 4 years ago

Yes, adding a test will certainly be a required part of the solution.

aminiussi commented 4 years ago

As for now, there still might be a problem with open MPI:

When including mpi.h, most the OpenMPI distribution I've found so far insist on detecting a C++ compiler, thus including the legacy C++ headers, which in turn brings in some (unused) C++ MPI symbols.

And I think OpenMPI is the most widespread MPI implementation.

I'm planing to explicitly prohibit that by default (#73) but untill then... (I'll try to do it this we at least on develop).

I have no experience with boostorg/boost_install yet.

pdimov commented 4 years ago

I'll transfer this issue to boost_install.

pdimov commented 4 years ago

As far as I can see, what's needed is https://github.com/boostorg/boost_install/commit/63c74f64562b4789781ab03d59ae38f81e70b9f8.

It's not possible to use PUBLIC for an imported library, only INTERFACE linking is allowed, but I hope CMake does the right thing with those.

I don't have a way to test that at present; it's on the feature/add-mpi-dependency branch. I'll merge it to develop upon confirmation that it works.

sdebionne commented 4 years ago

I had a look at Boost.Iostreams that also have dependencies to third party libraries (zlib, bzip2) and it looks like it might have the same issue. I have open an issue boostorg/boost_install#18 (in parallel to your issue transfer).

It's not possible to use PUBLIC for an imported library, only INTERFACE linking is allowed, but I hope CMake does the right thing with those.

I think it does the right thing.

pdimov commented 4 years ago

Looking at the documentation of FindMPI,

pdimov commented 4 years ago

Yes, find_dependency(MPI) finds just MPI_CXX for me by default, and does not declare MPI::MPI_C.

pdimov commented 4 years ago

Almost done; the MPI tests just fail because of https://github.com/boostorg/mpi/commit/8619d7b24a7b6ff7d6655ee0b10ef7fb2a470480#commitcomment-35672247.

pdimov commented 4 years ago

Merged to develop.

sdebionne commented 4 years ago

@pdimov Thank you very much! I'll try to find some time to contribute a test.

pdimov commented 4 years ago

It already has a test: https://github.com/boostorg/boost_install/tree/develop/test/mpi