Skip to content
This repository was archived by the owner on Sep 30, 2022. It is now read-only.

Conversation

@rhc54
Copy link

@rhc54 rhc54 commented Apr 30, 2016

Thanks to Orion Poplawski for pointing it out

The patch for master will likely be different as we are dealing with multiple versions, so this is just for 2.x for now.

…ib64 locations

Thanks to Orion Poplawski for pointing it out
@rhc54 rhc54 added this to the v2.0.0 milestone Apr 30, 2016
@mellanox-github
Copy link

Test PASSed.
See http://bgate.mellanox.com/jenkins/job/gh-ompi-release-pr/1578/ for details.

@rhc54
Copy link
Author

rhc54 commented Apr 30, 2016

Fixes open-mpi/ompi#1609

@ggouaillardet
Copy link
Contributor

@rhc54 i made open-mpi/ompi#1612, using OPAL_CHECK_PACKAGE macro makes it much easier to me

@jsquyres
Copy link
Member

jsquyres commented May 2, 2016

#1612 does seem quite a bit simpler, and uses the macro as intended.

@rhc54 Is there a reason not to use OPAL_CHECK_PACKAGE?

@ggouaillardet
Copy link
Contributor

fwiw, pmix used to use more include dirs than necessary, and hence OPAL_CHECK_PACKAGE was not a good fit. I reduced that to one include directory in order to use the macro.

@rhc54
Copy link
Author

rhc54 commented May 2, 2016

@ggouaillardet Did you read the comment explaining why we cannot use check_package??

            # cannot use check_package because there are
            # external dependencies to make the headers
            # build, so just check for presence of header
            # and library files - these checks will error
            # out if the files aren't found, which is okay
            # as we are only executing here if the user
            # specified external pmix

@ggouaillardet
Copy link
Contributor

I probably read the comment, but could not find such dependencies
(e.g. with my changes, it works just fine, with lib pmix,so in /lib or /lib64)

@rhc54
Copy link
Author

rhc54 commented May 2, 2016

Yes - IF and only if the external dependencies are all in standard locations! We tried this approach and had to reject it because it fails if things are in non-standard locations since you cannot know what PMIx linked itself against, and where those packages are located.

@jsquyres
Copy link
Member

jsquyres commented May 2, 2016

@ggouaillardet I think @rhc54 is saying that pmix.h includes other headers. If those headers are not in a standard location, then the include test fails (and it gets complicated/undesirable to try to list all the -I flags for the PMIx header dependencies just for this test).

@jsquyres
Copy link
Member

jsquyres commented May 2, 2016

It's ugly, but it works. 👍

@ggouaillardet
Copy link
Contributor

OpenMPI configure detects libevent and hwloc before pmix, and hence CPPFLAGS and LDFLAGS are already set when external pmix is checked.
am I right ?
if yes, what can possibly go wrong ?
or let me put it this way, if OPAL_CHECK_PACKAGE fails but current mechanism works, can we ensure 100% everything will work just fine at runtime ?
fwiw, in my vm , libevent is in a standard location, but pmix is in a non standard one

@rhc54
Copy link
Author

rhc54 commented May 2, 2016

@jsquyres and I just spent a bunch of time on the phone over this, and I think we concur that using OPAL_CHECK_PACKAGE should be okay for now. The reason we can do it is that check_package only does AC_TRY_LINK - it doesn't actually try to run anything, and thus the pmix external linkages don't matter so long as the header for those external links aren't directly referenced in the pmix headers.

So I will close this one out and we can use yours. Thx!

@rhc54 rhc54 closed this May 2, 2016
@rhc54 rhc54 deleted the cmr20/pmixext branch May 2, 2016 15:52
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants