-
Couldn't load subscription status.
- Fork 928
Cover the use-cases for OPAL_PREFIX and PMIX_INSTALL_PREFIX options #4012
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.
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
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This looks great. I'd wait for the other review before merging, but I think this does what we outlined. Thanks!
orte/orted/orted_submit.c
Outdated
| opal_unsetenv(OPAL_MCA_PREFIX"pmix", &orte_launch_environ); | ||
| /* clear any install_prefix that might have been set so | ||
| * we don't cause our children to falsely generate warnings */ | ||
| opal_unsetenv("PMIX_INSTALL_PREFIX", &orte_launch_environ); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Question:
Do we want to clear PMIX_INSTALL_PREFIX here? What if it was set in the user's environment to point to the correct external PMIx install (assume they are also configured Open MPI with an external PMIx). Wouldn't the children need this value to be set when they are launched as well?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
That's fine - it was being cleared to avoid conflicts on the internal version if OPAL_PREFIX was set and we then set PMIX_INSTALL_PREFIX in the daemon, thus delivering both to the child.
…hes between location directives for OPAL and PMIx. Provide a more helpful error message and error out if we find a mismatch. If any OPAL values are set and the PMIx equivalent is not, then transfer it. Do not clear PMIX_INSTALL_PREFIX from the daemon's launch environment Fixes #3980 Closes #4007 Refs #3985 Signed-off-by: Ralph Castain <rhc@open-mpi.org>
|
@artpol84 This look okay to you too? |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
👍
| } | ||
| OPAL_LIST_DESTRUCT(&values); | ||
| return OPAL_SUCCESS; | ||
| } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'd probably do else here
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ohh, it's a return - never mind then!
Fixes #3980
Closes #4007
Refs #3985
Signed-off-by: Ralph Castain rhc@open-mpi.org