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.

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

Fix some issues with tool help #1257

Merged
merged 1 commit into from
Mar 9, 2022
Merged

Conversation

awlauria
Copy link
Contributor

@awlauria awlauria commented Mar 9, 2022

  • Add missing PMIX_ERR_SILENT -> PRTE_ERR_SILENT. This was
    causing the prte command parsing to print an extra line with -h.

  • Pass the helpdir to pmix_init_util().
    This will trickle down the pmix_show_help_init(), which will
    be added to the pmix_show_help search path. Otherwise, PMIx
    will have no way to know where the PRTE tools help files
    are.

@awlauria
Copy link
Contributor Author

awlauria commented Mar 9, 2022

Wont build until openpmix/openpmix#2495 is merged.

@rhc54
Copy link
Contributor

rhc54 commented Mar 9, 2022

Hmmm...this isn't correct in terms of how you are handling the pmix_tool_basename. Something has been removed or else not committed yet from one of my branches. We set pmix_tool_basename = prte_tool_basename in prte_init_util so we can pickup the tool name being defined in the tool itself. You definitely do not want to hardwire schizo/ompi to mpirun - e.g., the user might be typing mpiexec, and that would get totally confusing.

I'll look at the basename problem in a few minutes - probably just part of some branch of mine that didn't get committed yet. Have too many branches running around right now.

@awlauria
Copy link
Contributor Author

awlauria commented Mar 9, 2022

Yeah, working on a fix for mpiexec - realized that after I posted the patch.

@awlauria
Copy link
Contributor Author

awlauria commented Mar 9, 2022

if you have a fix for that somewhere, I can remove that patch. I think we'll still need the helpdir fix, unless that is also somewhere else

@rhc54
Copy link
Contributor

rhc54 commented Mar 9, 2022

if you have a fix for that somewhere, I can remove that patch. I think we'll still need the helpdir fix, unless that is also somewhere else

I do and will provide it - yes, we still need the helpdir fix.

- Pass the helpdir to pmix_init_util().
  This will trickle down the pmix_show_help_init(), which will
  be added to the pmix_show_help search path. Otherwise, PMIx
  will have no way to know where the PRTE tools help files
  are.

- Add missing PMIX_ERR_SILENT -> PRTE_ERR_SILENT. This was
  causing the prte command parsing to print an extra line with -h.

Signed-off-by: Austen Lauria <awlauria@us.ibm.com>
@awlauria
Copy link
Contributor Author

awlauria commented Mar 9, 2022

Ok - repushed with that removed. I left the conversion for PRTE_ERR_SILENT for now.

@rhc54
Copy link
Contributor

rhc54 commented Mar 9, 2022

Yeah, that all makes sense. It occurs to me that while the PMIx constants are "fixed" by the standard, there is nothing constraining the PRRTE ones. Perhaps one solution to all this is for us to simply change the various PRRTE constants to have the same numerical value as their PMIx equivalents? Would avoid all this conversion stuff and eliminate the problems of being careful to check the right status version.

There are PRRTE constants that don't have a PMIx equivalent, but that would look just like an extension - i.e., define them to be something beyond the PMIx error constant boundary.

Make any sense?

@rhc54 rhc54 merged commit cd762eb into openpmix:master Mar 9, 2022
@awlauria
Copy link
Contributor Author

awlauria commented Mar 9, 2022

yeah - it makes sense to me to make them the same/align. I can work on that if you haven't already.

@awlauria awlauria deleted the awlauria_prte_help branch March 9, 2022 18:30
@rhc54
Copy link
Contributor

rhc54 commented Mar 9, 2022

I have not - if you can, that would be greatly appreciated! Afraid I'm rather bogged down in these debugger issues.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants