Skip to content

add libopen-orted-mpir when statically linking#9609

Merged
bwbarrett merged 1 commit intoopen-mpi:v4.1.xfrom
ggouaillardet:topic/orted_mpir
Nov 2, 2021
Merged

add libopen-orted-mpir when statically linking#9609
bwbarrett merged 1 commit intoopen-mpi:v4.1.xfrom
ggouaillardet:topic/orted_mpir

Conversation

@ggouaillardet
Copy link
Contributor

@ggouaillardet ggouaillardet commented Nov 1, 2021

This is a one-off commit for the v4 series since
v5 and later moved to prrte.
bot:notacherrypick

Thanks Matt Thompson for reporting this issue.

Signed-off-by: Gilles Gouaillardet gilles@rist.or.jp

This is a one-off commit for the v4 series since
v5 and later moved to prrte.

Thanks Matt Thompson for reporting this issue.

Signed-off-by: Gilles Gouaillardet <gilles@rist.or.jp>
@ggouaillardet ggouaillardet requested a review from rhc54 November 1, 2021 05:55
@ggouaillardet
Copy link
Contributor Author

@rhc54 could you please have a look at this PR?

Currently, if Open MPI is configure'd with --enable-static --disable-shared, linking a simple MPI program fails because some MPIR_* symbols are missing. This PR addresses that by adding libopen-orted-mpir to the list of static libs, but I am not quite sure if this is the right fix.

@rhc54 rhc54 removed their request for review November 1, 2021 14:41
@rhc54
Copy link
Contributor

rhc54 commented Nov 1, 2021

I'm not the correct person - haven't had anything to do with MPIR in ages.

@awlauria I think you folks deal with this now?

@awlauria
Copy link
Contributor

awlauria commented Nov 1, 2021

The change makes sense to me. @jsquyres / @gpaulsen / @bwbarrett ?

@hppritcha
Copy link
Member

@ggouaillardet would you mind opening an equivalent PR for the v4.0.x series?

@jsquyres jsquyres requested a review from bwbarrett November 1, 2021 20:22
@jsquyres
Copy link
Member

jsquyres commented Nov 1, 2021

@bwbarrett This looks correct to me. Can you think of any problems with it?

@bwbarrett
Copy link
Member

I can't think of anything; it raises the question why we're leaving it in a standalone library, but let's not worry about that for now.

@bwbarrett bwbarrett merged commit bc0d2b1 into open-mpi:v4.1.x Nov 2, 2021
@awlauria
Copy link
Contributor

awlauria commented Nov 2, 2021

It was compiled into its stand-alone library to compile it with its own CFLAGS, otherwise they would be compiled with -O3 and the like:

#8428

@ggouaillardet
Copy link
Contributor Author

the PR for the v4.0.x series is in #9618

@ggouaillardet
Copy link
Contributor Author

@awlauria thanks for the insight.
is there any reason why this library was not slurped into libopen-rte (instead of being a dependency)?

@ggouaillardet
Copy link
Contributor Author

@awlauria @bwbarrett I think I understand ... and I messed up (!)

the library used to be installed, but is now slurped (aka noinst) since 952328c3827

that means this PR should be reverted (!)

do you concur?

@jsquyres
Copy link
Member

jsquyres commented Nov 3, 2021

Ah shoot. Yes, there's the reason this PR shouldn't have been merged. Oh well; let's revert...

@jsquyres
Copy link
Member

jsquyres commented Nov 3, 2021

FWIW, I confirm with my own local build that the v4.1.x branch is now broken:

mpicc -g  hello_c.c  -o hello_c
/usr/bin/ld: cannot find -lopen-orted-mpir
collect2: error: ld returned 1 exit status

Why didn't CI catch this? Weird.

@bwbarrett
Copy link
Member

I'm not sure why CI didn't catch this. I'll dig into that; very weird.

@ggouaillardet
Copy link
Contributor Author

The issue only occurs if Open MPI is configured with --disable-shared --enable-static and a MPI wrapper is used. Not sure CI tests this.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants