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

Ensure C language when requiring MPI #5062

Open
wants to merge 1 commit into
base: develop
Choose a base branch
from
Open

Conversation

dg0yt
Copy link

@dg0yt dg0yt commented Nov 2, 2024

With PARALLEL enabled, the exported targets depend on MPI::MPI_C. However, this target isn't created by find_package(MPI) when the consuming project has only language CXX enabled. There is also no good diagnostics because MPI is found successfully.

(Alternative: Use find_package(MPI ... COMPONENTS C). This would enable proper diagnostics. But simply enabling language C is more effective. This might be reconsidered when fully supporting cmake config search by switching to find_dependency() instead of find_package(... REQUIRED).)

Noticed while testing vcpkg port gdal with hdf5 being a transitive dependency - the minimal test port had just CXX enabled.

This PR: Build successful.

Before this PR:

CMake Error at /Users/vcpkg/Data/installed/arm64-osx/share/hdf5/hdf5-targets.cmake:61 (set_target_properties):
  The link interface of target "hdf5::hdf5-static" contains:

    MPI::MPI_C

  but the target was not found.  Possible reasons include:

    * There is a typo in the target name.
    * A find_package call is missing for an IMPORTED target.
    * An ALIAS target is missing.

Diagnostics-only alternative:

CMake Error at /<redacted>/vcpkg/vcpkg/downloads/tools/cmake-3.30.1-linux/cmake-3.30.1-linux-x86_64/share/cmake-3.30/Modules/FindPackageHandleStandardArgs.cmake:233 (message):
  Could NOT find MPI (missing: MPI_C_FOUND C)

      Reason given by package: MPI component 'C' was requested, but language C is not enabled.  

@derobins derobins added Priority - 2. Medium ⏹ It would be nice to have this in the next release Component - Parallel Parallel HDF5 (NOT thread-safety) Component - Build CMake, Autotools Type - Bug / Bugfix Please report security issues to [email protected] instead of creating an issue on GitHub labels Nov 4, 2024
@byrnHDF
Copy link
Contributor

byrnHDF commented Nov 4, 2024

While I think this is just fine - the only worry is if there will be any unintended issues. I Can't think of any since the library is designed to be built with "C".

@dg0yt
Copy link
Author

dg0yt commented Nov 12, 2024

I acknowledge that there are some restrictions on where enable_language can be used:
https://cmake.org/cmake/help/latest/command/enable_language.html
I believe that these cases should be rare for normal use of find_package. But OTOH I have seen enough weird cmake code...

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Component - Build CMake, Autotools Component - Parallel Parallel HDF5 (NOT thread-safety) Priority - 2. Medium ⏹ It would be nice to have this in the next release Type - Bug / Bugfix Please report security issues to [email protected] instead of creating an issue on GitHub
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants