openmpi-4.1.6: ldflags in pkg-config incompatible with CMake's pkg_check_modules(... IMPORTED_TARGET ...)

[SomeoneSerge]

Thank you for taking the time to submit an issue!

Background information

What version of Open MPI are you using? (e.g., v3.0.5, v4.0.2, git branch name and hash, etc.)

v4.1.6

❯ nix eval github:NixOS/nixpkgs/91a00709aebb3602f172a0bf47ba1ef013e34835#openmpi.version
"4.1.6"

Describe how Open MPI was installed (e.g., from a source/distribution tarball, from a git clone, from an operating system distribution package, etc.)

nix build github:NixOS/nixpkgs/91a00709aebb3602f172a0bf47ba1ef013e34835#openmpi

Details of the problem

I'm not sure if this has been addressed on master yet (I don't think it has), but in v4.1.6 the generated .pc files' Libs contain a number of occurrences of the pattern -Wl,-rpath -Wl,... where the argument to -rpath is passed on as a separate token. CMake only keeps the first -Wl,-rpath. This issue can be alleviated by passing the the paths as -Wl,-rpath,${libfabricPath}

❯ nix build github:NixOS/nixpkgs/91a00709aebb3602f172a0bf47ba1ef013e34835#openmpi
❯ cat result/lib/pkgconfig/ompi-c.pc
...
Libs: ...   -Wl,-rpath -Wl,${libdir}  -Wl,-rpath -Wl,/nix/store/iyzz6m4dr6grn10qi67a1g1lp3wrnk7j-libfabric-1.20.0-dev/lib ...

Cf. also https://discourse.cmake.org/t/issues-with-opmi-cxx-pc-s-ldflags-and-pkg-check-modules-imported-target/9651

Thanks

[jsquyres]

Are you asking about v4.1.2 or v4.1.6? Your text mentions both.

If you're asking about v4.1.2, can you test with v4.1.6? Many things have been fixed since v4.1.2.

[SomeoneSerge]

Sorry, that slipped through. The logs I had attached were for v4.1.6 and I just updated the description accordingly. To be clear, this isn't exactly an openmpi's issue (more like CMake not being able to handle all the cases of the functionality it claims to provide) per se, but in practice it's desirable for openmpi to adjust

[jsquyres]

Is it universally supported that

-Wl,-rpath -Wl,/some/dir

is equivalent to

-Wl,-rpath,/some/dir

?

I ask because I don't think we've ever used the form -Wl,-rpath,/some/dir. I don't know if there's a reason for that, or if we've used the other form simply because we've always used the other form.

Alternatively, is there a reason Cmake only uses the first token? Why doesn't it use all of Libs?

[SomeoneSerge]

I ask because I don't think we've ever used the form -Wl,-rpath,/some/dir

I don't know about that but I see some matches:

ompi/.ci/community-jenkins/pr-builder.sh

Line 192 in bd33b99

CONFIGURE_ARGS="$CONFIGURE_ARGS LDFLAGS=-Wl,-rpath,/usr/local/lib/gcc5 --with-wrapper-ldflags=-Wl,-rpath,/usr/local/lib/gcc5"

Alternatively, is there a reason Cmake only uses the first token? Why doesn't it use all of Libs?

I'm not really familiar with CMake internals, but I can refer you to this reply: https://discourse.cmake.org/t/issues-with-opmi-cxx-pc-s-ldflags-and-pkg-check-modules-imported-target/9651/2?u=someone

Is it universally supported that

I think so but it'll take some research to confirm. Looking at man 1 ld for the GNU ld, there is a warning about prefixing args with -Wl when invoking the linker "indirectly", and about whitespaces. Should also be equivalent to -Wl,-rpath=...?

This is important, because otherwise the compiler driver program may silently drop the linker options, resulting in a bad link. Confusion may also arise when passing options that require values through a driver, as the use of a space between option and argument acts as a separator, and causes the driver to pass only the option to the linker and the argument to the compiler. In this case, it is simplest to use the joined forms of both single- and multiple-letter options, such as:

            gcc foo.o bar.o -Wl,-eENTRY -Wl,-Map=a.map

[jsquyres]

pr-builder.sh

Ah, that's just a CI script -- we don't ship that. That's only used in a specific set of circumstances (vs. configure, which has to work everywhere).

Cmake only using the first token

From their reply, it looks like cmake isn't using just the first token, it does a de-duplication of all the tokens in Libs, and therefore all but one instance of -Wl,-rpath get removed. This then leads to Badness.

ld(1)

Ok. Will need to do some research here; the -Wl,-rpath -Wl,/some/dir form has worked for many years, and therefore supports both very old and modern systems. What we'd need to check is if -Wl,-rpath=/some/dir and/or -Wl,-rpath,/some/dir forms work on the oldest systems that we still care about.

I suppose a reasonable proxy might be to try on a CentOS 7 box (which is just about to go EOL in a few months), and possibly also an Ubuntu 18 box (which is already EOL, but if 2 alternate forms work there, then they probably work on all platforms that we still care about).

[SomeoneSerge]

From their reply, it looks like cmake isn't using just the first token, it does a de-duplication of all the tokens in Libs, and therefore all but one instance of -Wl,-rpath get removed

Yes, I got that much as well

Ah, that's just a CI script -- we don't ship that.
...
Will need to do some research here;

Maybe these would speed up the process, e.g. I see that the Darwin branches in contrib files come with some extra comments

❯ ag --hidden -- '-Wl,-rpath[,=]'
contrib/platform/lanl/darwin/mic-common
33:MIC_LDFLAGS="-L$XCOMPOSER/compiler/lib/mic -Wl,-rpath=$XCOMPOSER/compiler/lib/mic"

contrib/dist/linux/buildrpm.sh
260:        configure_options="$configure_options --with-libfabric=$libfabric_path \"LDFLAGS=-Wl,--build-id -Wl,-rpath,$libfabric_path/lib64 -Wl,--enable-new-dtags\""
265:        configure_options="$configure_options --with-libfabric=$libfabric_path \"LDFLAGS=-Wl,--build-id -Wl,-rpath,$libfabric_path/lib -Wl,--enable-new-dtags\""

3rd-party/romio341/confdb/aclocal_shl.m4
117:    # As of 10.5, -Wl,-rpath,dirname should work .  The dirname 
119:    # -Wl,-rpath,path for each of the paths in the list).  However, os x
148:        C_LINKPATH_SHL="-Wl,-rpath,"
257:        C_LINKPATH_SHL="-Wl,-rpath,"

3rd-party/romio341/mpl/confdb/aclocal_shl.m4
117:    # As of 10.5, -Wl,-rpath,dirname should work .  The dirname 
119:    # -Wl,-rpath,path for each of the paths in the list).  However, os x
148:        C_LINKPATH_SHL="-Wl,-rpath,"
257:        C_LINKPATH_SHL="-Wl,-rpath,"

.ci/community-jenkins/pr-builder.sh
192:        CONFIGURE_ARGS="$CONFIGURE_ARGS LDFLAGS=-Wl,-rpath,/usr/local/lib/gcc5 --with-wrapper-ldflags=-Wl,-rpath,/usr/local/lib/gcc5"

also ag --hidden -- '-Wl,[a-zA-Z0-9-]+[,=]', and the github code search (matches seem to include pytorch, tf, and the kernel, which is promising; matches for -Wl,-rpath= include cpython, golang and git)

[jsquyres]

Most of what you found via ag in the OMPI tree are secondary scripts that are only run in specific cases. That being said, I found those romio scripts as well; I'd have to check the context there, but it's possible that that implies it is doing -Wl,-rpath,/path in the general case -- i.e., it fits the "it runs everywhere" criteria.

The data points for -Wl,-rpath=/blah is also quite promising; thanks!

[jsquyres]

Ugh. I'm remembering the issue here. Open MPI is not supplying those -Wl,-rpath -Wl,/path arguments -- we're directly extracting them from the Libtool that was generated for this platform.

Honestly, this is a CMake bug. Using -Wl,-rpath -Wl,/path in a .pc file's Libs entry is perfectly valid. If CMake wants to deduplicate Libs, it should do it carefully / correctly -- or not at all. Asking systems that are 2 layers down from CMake to fix the issue is not really the right solution.

[SomeoneSerge]

Honestly, this is a CMake bug.

Absolutely, that's the message I started from 😅. I'll open a ticket in https://gitlab.kitware.com/cmake/cmake/-/issues as well as soon as I can catch a break

That said, I think playing well with CMake is fairly important even when CMake is wrong. Depending on how hard it is to rewrite libtool's flags it might be worth consideration...

[ggouaillardet]

This is getting absurd: there are two main MPI implementations (MPICH and Open MPI) and the other ones are likely derivatives, so if CMake is serious about MPI, they should support both without asking us to change anything.

Bonus point, fixing CMake will work with existing Open MPI implementations, not only the future ones.

[SomeoneSerge]

Hi! This isn't about how CMake supports MPI (e.g. this is unrelated to their FindMPI.cmake, tracked separately), this is about CMake's generic pkg-config support. It's just another consumer for the flags in .pc files same as gcc or clang, and this one currently happens to not understand the space-separated linker flags (we have not found consumers who do not support the comma- or =-joined arguments yet). As I said, I'm going to file an issue upstream, only I expected that it'll take longer to process and solve (it's likely easier to write things in a simpler format, than to extend the generic parser to support one more special case)

If ompi isn't interested in supporting the (specific invocation of) pkg_check_modules that's a valid call. Shall I close the issue?