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

Initiate move of CLI help files to common location #1765

Merged
merged 3 commits into from
Sep 10, 2023

Conversation

rhc54
Copy link
Contributor

@rhc54 rhc54 commented Jun 16, 2023

In order to make it easier for 3rd-parties to automate the assembly of their own help docs, move the CLI help files to a new common location. Retain their names so we don't have to go back and update everywhere they are used, and ensure they still get installed to the proper location that show_help is expecting.

@rhc54
Copy link
Contributor Author

rhc54 commented Jun 16, 2023

@jsquyres This PR moves two CLI help files into a new common location at src/docs. I also realized that I have done very little with the PRRTE documentation beyond the CLI help text, focusing my time more on the PMIx side of things. I'll hold off doing anything on it until we know the final results of our chat.

@jsquyres
Copy link
Contributor

jsquyres commented Jul 4, 2023

Rebased to fix conflicts.

@jjhursey
Copy link
Member

jjhursey commented Jul 5, 2023

bot:ibm:pgi:retest

@jsquyres
Copy link
Contributor

jsquyres commented Jul 11, 2023

Status report: much has been done, but there's still more to do.

  • Three .txt files were 1:1 converted to .rst files.
  • First cut of Sphinx plugin is written that renders new, custom RST directives in the .rst files to HTML+nroff and txt (i.e., INI-style show_help file) -- it all mostly works (see below for one case that isn't quite right yet).
  • Configury and sym links are in place to make this all work
  • make distcheck works
  • the rendered output is all (mostly) as expected.

Still to do:

  • Fix Sphinx prterun-section plugin glitch that does not return to the proper doc level when exiting a section with subsections
  • Re-order content in the newly-created RST files so that when rendered to HTML/nroff, they tell a cohesive story
  • Finish corresponding Open MPI side of things:
    • Slurp in PRTE RST files to make cohesive mpirun.1 RST
    • Make cohesive header / footer / other sections in mpirun.1 RST to tell a complete mpirun man page story
    • Add sym links / configury / whatever to find these RST files for the embedded PRTE
    • For external PRTE:
      • First cut: make the rendered mpirun.1 RST files say "You built with an external PRTE. The information here may be different yadda yadda yadda", and then slurp in the RST files for the embedded PRTE
      • Extra bonus points: slurp in the installed PRTE RST files (instead of the RST files for the embedded PRTE).
      • Be sure to handle the case where the external PRTE does not have the RST files (e.g., older PRTE versions).
    • Ensure that distcheck passes, and that the rendered HTML+man pages for mpirun are correct

I'm probably the one that needs to fix the Sphinx plugin, but assistance on the other steps would be great.

@jsquyres
Copy link
Contributor

I have just filed a corresponding (draft) OMPI PR to do the OMPI side of this work: open-mpi/ompi#11820

@jsquyres
Copy link
Contributor

jsquyres commented Jul 18, 2023

@rhc54 I don't think this is done yet, and I'm sure there are still content / flow errors. But it contains most of what we talked about today. What do you think? In particular, look at these 4 files:

@rhc54
Copy link
Contributor Author

rhc54 commented Jul 18, 2023

I renamed this from help-schizo-detail.rst, which may not have been a good idea (is there code that refers to help-schizo-detail.txt?)

My original plan was to collect all the sections that are strictly indirectly referenced in that file - hence the generic name. However, that plan no longer fits what we discussed, so renaming it is fine. Would have to rename some places that refer to the "detail" name, but I'm going to have to do that regardless, I think - not clear to me that we will wind up replicating names across the board. Thankfully, it can be done with a global search/replace.

What do you think?

Pretty much looks like what we described. The placement-deprecated section is what you would now call the deprecated-map-by, which seems to have been split into a variety of deprecated files, each for one deprecated CLI option?

I guess it follows the pattern of what you did for each CLI option. I can grok the idea - make it so atomistic that someone can pick/choose the individual options they want to use. I just hadn't taken what we discussed and gone to that level of atomicity. No real objection to it - just a tad more than I was expecting.

Given that, I'm not sure I understand the need for src/docs/help-schizo-cli.rst. The output from that file (and the others like it that you cite) don't seem to "fit" any more. Instead, I would need to create a src/mca/schizo/prte/help-schizo-prterun.rst file that has the overview in it and then all the "include" directives for the individual options. This would then generate the corresponding text file used by show-help. No reason for me to include directives that reference a file that contains all the generic entries - just duplicate work (create the generic file and then create a top-level file that references them).

Is that correct? Or am I missing something?

@rhc54
Copy link
Contributor Author

rhc54 commented Jul 18, 2023

Couple of thoughts. I realized that the reason for your current files was to minimize the near-term conversion impact on me. However, I think it creates a long-term coherence issue. If we are going to convert the show-help CLI system over to RST, then we probably should bite the bullet and do it. If all I have to do is convert the top-level CLI help text files in the schizo/prte component, that isn't such a big deal. Would be much worse if we were converting all the show-help error output - maybe someone can someday do it, but we should avoid that for now.

My other thought is for extensibility. Today we limit the help CLI system to the option, but no suboption - i.e., you can do prterun --help display but you cannot do prterun --help display alloc. This is a limitation I would like to remove fairly soon. We'd like to be able to do something like provide more detail on that specific suboption, perhaps including example output.

I don't believe your proposed system would preclude this. We might do something like add a display-alloc entry somewhere and update the show-help function to concatenate the option and suboption to create the lookup tag. Since the goal would be to provide more detail than just --help display would produce, I wouldn't expect --help display to include a link to the display-alloc entry. I suppose there would be no harm in including the display alloc detail on the web site if it was desirable.

Just raising it here in case you see something that would preclude that extension.

@jsquyres
Copy link
Contributor

jsquyres commented Aug 16, 2023

@rhc54 Per our Slack conversation, I agree with everything you mentioned above.

I converted the rest of the help-<cmd> show help files from src/mca/schizo/prte, and put them in src/docs. I then converted all their #include directives to be RST .. include:: directives. I think it's all working -- can you give it a look?

Also, I was having difficulty in getting all the individual files from src/docs/SINGLE_TOPIC.rst included properly in docs/man/man1/COMMAND.rst (because some of the SINGLE_TOPIC.rst files include other SINGLE_TOPIC.rst files, and the include paths are different). I finally gave up and sym linked all the SINGLE_TOPIC.rst files into docs/man/man1/prterun, just in an effort to get this done/working (on the assumption that I could do something cleaner later).

But then I had a thought: why not make the docs/man/man1/COMMAND.rst files be very bare-bones, saying essentially "You should look at COMMAND --help for all the help about this command." See https://prrte--1765.org.readthedocs.build/en/1765/man/man1/prterun.1.html, for example.

What do you think of that idea?

If you're ok with all of this, I think this side of the work is getting close to done.

  1. Probably need to check/audit all the calls to show_help(...) here in PRTE because I'm guessing I wasn't careful and probably changed some filenames (the topic names are probably ok, though...?).
  2. I need to refresh Include PRTE RST content in mpirun.1 man page open-mpi/ompi#11820 because things changed here a bit

@jsquyres
Copy link
Contributor

What do you think of that idea?

Turns out that this isn't the greatest idea, because all the same problems with including content in PRTE man pages come up in pretty much the same way when trying to include content in OMPI man pages.

Meaning: it's worth solving those issues here in PRTE, and then replicating that solution in OMPI. Working on it...

Signed-off-by: Jeff Squyres <[email protected]>
Remove long-since commented out code.

Signed-off-by: Jeff Squyres <[email protected]>
@jsquyres jsquyres force-pushed the topic/rst branch 2 times, most recently from 8b66091 to f82ccfd Compare September 10, 2023 15:52
jsquyres added a commit to jsquyres/ompi that referenced this pull request Sep 10, 2023
This commit is just for testing / CI purposes on the PR.

It updates the prrte submodule to the head of the topic/rst PRTE
banch, which is openpmix/prrte#1765.

Once this PR is merged, we should update the submodule in this PR to
be the head of the PRTE master branch (not the PR branch).

Signed-off-by: Jeff Squyres <[email protected]>
jsquyres added a commit to jsquyres/ompi that referenced this pull request Sep 10, 2023
This commit is just for testing / CI purposes on the PR.

It updates the prrte submodule to the head of the topic/rst PRTE
banch, which is openpmix/prrte#1765.

Once this PR is merged, we should update the submodule in this PR to
be the head of the PRTE master branch (not the PR branch).

Signed-off-by: Jeff Squyres <[email protected]>
This commit represents a major reorganization and revamp of prterun's
show_help files.  The overriding purpose is to allow downstream
projects -- such as MPI	implementations	-- to include PRRTE's Sphinx
(.rst) documentation in their own.

This entailed the following:

* The existing "show help" .txt files are used by the existing "<cmd>
  --help <topic>" CLI system.  It was important to preserve this CLI
  capability.

* However, we wanted this same content to also appear in various man
  pages (and therefore also HTML documentation).  Hence, we moved the
  existing "show_help" .txt files to a new location: src/docs/
  (vs. docs/, where the main PRRTE documentation lives).

  * After various experiments, we determined that we just could not
    have both the main PRRTE Sphinx docs and this new class of
    show_help/man-page-content combination docs in the same Sphinx
    docs tree.  The structure and layout of how we need to render
    these two sets of docs are quite different.

    Instead, as described below, we took a different approach: have
    "building blocks" of content that can be assembled in different
    ways that are ultimately rendered with the structure and layout
    required for each environment.

* src/docs/ contains 2 directories:

  * prrte-rst-content: this directory contains a README.txt file that
    both PRRTE docs authors and downstream consumers of the RST
    content should read.  It explains how both how to author/maintain
    the docs in that directory, as well as how to integrate the
    contents of this directory into downstream project RST docroots,
    and how to include the content.

    Essentially, this directory is the main source of prterun CLI
    documentation content.  Each topic is split into its own file (see
    the README for more info) so that it can be RST-included in other
    .rst files as relevant.  Think of these files as individual
    building blocks of content that can be assembled in multiple
    different ways.

    The text content in this directory is mostly content from previous
    show_help-style text files moved into this directory, split into
    individual .rst files, and then converted to RST syntax.  Some of
    the tables ended up being nicer / easier to maintain, for example.

    We install these .rst files to the installation tree so that
    downstream projects can use them in their own documentation.

  * show-help-files: the contents of this directory are rendered to
    what ultimately become the "show_help" text files.

    This directory effectively RST-includes the files from the
    prrte-rst-content directory into files and renders them as
    INI-style "show_help" text files (i.e., suitable for reading by
    the prte_show_help() subsystem).

* The src/docs/prrte-rst-content is also RST-included in the top-level
  docs/ .rst files (va a sym link).  That allows the same "building
  blocks" of documentation content to be included in both the
  show_help files and the HTML/man pages.

* The Open MPI schizo component ("ompi") also installs its own .rst
  files into the installation tree so that Open MPI can include this
  content in its documentation.

* Note that in distribution tarballs, pre-built HTML docs, norff man
  pages, and INI-style "show_help" text files will be included in the
  tarball -- end users do not need to have Sphinx available.

  For git clones, where none of the Sphinx-generated content is
  available (i.e., HTML docs, nroff man pages, show_help text files),
  Sphinx is also still not required (but it *is* still required to
  "make dist").  If Sphinx is not available when "configure" is
  invoked, dummy INI-style "show_help" text files are generated (that
  basically say "You don't have Sphinx, so you don't get help here").

That is the overall scheme.  There were many other minor points
included in this work, including (but not limited to):

* Moving existing explanations of hosts and hostfiles -- and expanding
  on them -- to the main docs.

* Moving explanations of relative indexting to the main docs.

* Moving explanations of notifications to the main docs.

* Moving listing of deprecated CLI options to the main docs.

* Moving explanations of diagnostics to the main docs.

* Moving most process placement explanations and examples to the main
  docs.

* Moving explanations of the session directory to the main docs.

* Adding psched.1 and pterm.1 man pages.  More work needs to be done
  here.

* Slightly improve prte.1, prte_info.1, prted.1, prterun.1, and prun.1
  man pages.  More work needs to be done here.

It is expected that the text content of all the docs will continue to
be improved over time.  The focus of this commit was not to make the
content perfect, but rather to create a first-gen infrastructure for
having a single source of documentation content that can be rendered
by Sphinx in 3 ways (HTML docs, nroff man pages, and show_help files),
and also included in downstream RST documentation.

Signed-off-by: Ralph Castain <[email protected]>
Signed-off-by: Jeff Squyres <[email protected]>
@jsquyres jsquyres marked this pull request as ready for review September 10, 2023 20:48
@jsquyres
Copy link
Contributor

After talking with @rhc, we agreed to merge and fix up any translation / content errors once it's in.

@jsquyres jsquyres merged commit 1e4edb5 into openpmix:master Sep 10, 2023
2 checks passed
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.

3 participants