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

Extract 3rd party configure options for OMPI configure #8409

Merged
merged 2 commits into from
Mar 5, 2021
Merged

Extract 3rd party configure options for OMPI configure #8409

merged 2 commits into from
Mar 5, 2021

Conversation

jjhursey
Copy link
Member

@jjhursey jjhursey commented Jan 23, 2021 •
edited
Loading

@jjhursey jjhursey mentioned this pull request Jan 23, 2021
Closed
@jjhursey
Copy link
Member Author

Details on the design are here

Left to do:

  • Re-enable this for hwloc and libevent
  • Refine the exclude file for all 3rd party projects

@bwbarrett
Copy link
Member

The revert should definitely be committed, as that patch should really never have been added. ./configure --help=recursive is the proper way to handle subconfigure options. We shouldn't be inventing our own option.

I obviously think the scripts to replicate the behavior of ./configure --help=recursive should not be committed, for the same reason.

I'm curious the use case for having a different location of SLURM (or similar) for Open MPI and PRRTE. I talked to Ralph when doing the initial work, and we could not come up with one that seemed useful enough for that code (and the complexity of users accidentally specifying a package enablement to prrte but not pmix, for example. So I'd really rather we not add that functionality without a bunch more discussion.

@open-mpi open-mpi deleted a comment from ibm-ompi Jan 25, 2021
@jjhursey
Copy link
Member Author

Let's continue the discussion on the original ticket to reach a resolution: Issue #7486

bwbarrett
bwbarrett reviewed Feb 9, 2021
View reviewed changes
Copy link
Member

@bwbarrett bwbarrett left a comment

Choose a reason for hiding this comment

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

I only reviewed the first two patches, since the third (the renaming patch) won't be needed.

.gitignore Outdated Show resolved Hide resolved
3rd-party/exclude-config.ini Show resolved Hide resolved
autogen.pl Outdated Show resolved Hide resolved
autogen.pl Outdated Show resolved Hide resolved
autogen.pl Outdated Show resolved Hide resolved
config/opal_config_pmix.m4 Outdated Show resolved Hide resolved
@gpaulsen gpaulsen added this to the v5.0.0 milestone Mar 2, 2021
@jjhursey
Revert "Expose the PMIx/PRRTE unique configure args"
56c7ed4 
This reverts commit 540b2dc.

Signed-off-by: Joshua Hursey <jhursey@us.ibm.com>
@jjhursey jjhursey force-pushed the build-conf-3rd branch 3 times, most recently from 1f6181b to f12d819 Compare March 3, 2021 16:39
@open-mpi open-mpi deleted a comment from ibm-ompi Mar 3, 2021
@open-mpi open-mpi deleted a comment from ibm-ompi Mar 3, 2021
@open-mpi open-mpi deleted a comment from ibm-ompi Mar 3, 2021
@jjhursey
Copy link
Member Author

jjhursey commented Mar 3, 2021 •
edited
Loading

Left to do:

  • Audit the exclude-config.ini
  • Deduplicate configure options (see note here )
  • Squash commits

@jjhursey
Script to extract configure options from 3rd party libraries
48cb583 
 * Extracts configure arguments from 3rd party packages
 * Takes care to de-duplicate options already provided by OMPI
   - So there is a preference for the OMPI configure option with
     its help string.

Signed-off-by: Joshua Hursey <jhursey@us.ibm.com>
@jjhursey jjhursey marked this pull request as ready for review March 4, 2021 03:07
@jjhursey
Copy link
Member Author

jjhursey commented Mar 4, 2021

This PR is now ready for review. I believe that it has addressed all of the requirements that the community has defined for this functionality from Issue #7486.

@jjhursey
Copy link
Member Author

jjhursey commented Mar 5, 2021

Does anyone else want to review before I merge? Otherwise, I'll bring this in tomorrow.

@jjhursey jjhursey merged commit 9063110 into open-mpi:master Mar 5, 2021
@jjhursey jjhursey deleted the build-conf-3rd branch March 5, 2021 18:12
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@bwbarrett bwbarrett bwbarrett approved these changes

@jsquyres jsquyres Awaiting requested review from jsquyres

@rhc54 rhc54 Awaiting requested review from rhc54

Assignees
No one assigned
Labels
Target: v5.0.x
Projects
None yet
Milestone
v5.0.0
Development

Successfully merging this pull request may close these issues.

Re-enable runtime-enabled configure CLI params (to pass to PRRTE)
3 participants
@jjhursey @bwbarrett @gpaulsen