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.

Sign up for GitHub

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

Jump to bottom

v3.0: configury: revamp ucx detection #4398

Merged
merged 1 commit into from
Apr 10, 2018
Merged

v3.0: configury: revamp ucx detection #4398

merged 1 commit into from
Apr 10, 2018

Conversation

ggouaillardet
Copy link
Contributor

@ggouaillardet ggouaillardet commented Oct 25, 2017 •
edited
Loading

  • when --with-ucx=DIR is not set, try the default path and fallback to /opt/ucx
  • when --with-ucx-libdir is not set, try lib64 and then lib directories
  • do not handle --with-ucx-libdir (this is a user mistake, no need to over-complicate our logic)

Refs. #4345

Signed-off-by: Gilles Gouaillardet [email protected]

(cherry picked from commit af03f55)

@ggouaillardet
configury: revamp ucx detection
c98b309 
 - when --with-ucx=DIR is not set, try the default path and fallback to /opt/ucx
 - when --with-ucx-libdir is not set, try lib64 and then lib directories
 - do not handle --with-ucx-libdir (this is a user mistake, no need to over-complicate our logic)

Signed-off-by: Gilles Gouaillardet <[email protected]>

(cherry picked from commit open-mpi/ompi@af03f55)
@ggouaillardet ggouaillardet added this to the v3.0.1 milestone Oct 25, 2017
@hppritcha
Copy link
Member

hmmm, now its OS-X.
bot:ompi:retest

@jsquyres
Copy link
Member

Per #4383 (comment), I'm confused by the commit message on this PR.

@alinask
Copy link
Member

alinask commented Oct 26, 2017

@ggouaillardet Can you please comment on why you are setting /opt/ucx as the fallback?

@ggouaillardet
Copy link
Contributor Author

The current version already fallback to /opt/ucx. I did not write this, and i did not change that behavior.

@ggouaillardet
Copy link
Contributor Author

@karasevb since you introduced this in d917d54 can you please answer @alinask ?

@karasevb
Copy link
Member

@alinask the main idea of the original PR #3962 was the added UCX autodetection by default (and also for case when the UCX path was not defined when configure) on the specified paths:

  1. discovered by pkg-config
  2. /usr
  3. /usr/local
  4. /opt/ucx this is one of the possible path where UCX can be found by default

@ggouaillardet meant that configure will look for UCX in /usr:/usr/local and then /opt/ucx, please correct me if this is not true.

@jsquyres jsquyres changed the title configury: revamp ucx detection v3.0: configury: revamp ucx detection Jan 5, 2018
@bwbarrett bwbarrett modified the milestones: v3.0.1, v3.0.2 Mar 1, 2018
@hppritcha
Copy link
Member

@jladd-mlnx please review. we hit this on 2.1.3 at LANL and its very annoying (issue #4345 I mean). I had to ask the sysadmin to do a hack LDFLAG workaround on the configure line.

I'd like to patch 2.1.X with this patch if its accepted.

@hppritcha
Copy link
Member

@xinzhao3 please ask Josh to review this PR.

jladd-mlnx
jladd-mlnx approved these changes Apr 3, 2018
View reviewed changes
Copy link
Member

@jladd-mlnx jladd-mlnx left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👍

@jladd-mlnx
Copy link
Member

@hppritcha good to go.

@bwbarrett bwbarrett merged commit 68f0576 into open-mpi:v3.0.x Apr 10, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@jladd-mlnx jladd-mlnx jladd-mlnx approved these changes

Assignees

@jladd-mlnx jladd-mlnx

Labels
bug Target: v3.0.x
Projects
None yet
Milestone
v3.0.2
Development

Successfully merging this pull request may close these issues.
7 participants
@ggouaillardet @hppritcha @jsquyres @alinask @karasevb @jladd-mlnx @bwbarrett