Asciidoctor to ReST doc conversion #3033
Conversation
gonsie
changed the title
|
Thanks! I can try to help integrate sphinx or whatever is required on the build system side of things. You will also need to remove Did COPYRIGHT inclusion get dropped on purpose in man pages? (I notice some other includes were preserved) |
There was a problem hiding this comment.
Thanks @gonsie for putting this together! You rock!
Did a quick pass (haven't run/compiled it yet) and it looks good! One minor comment below.
|
@grondo This PR definitely was a rough start and I wasn’t sure the exact use-case that would need to be supported. I think the basic use case is: If the ‘enable docs’ option is on, then the docs should be built and installed with The copyright statement at the bottom of the man pages is generated & inserted by sphinx. Actually, the author statement is inserted as well, I’ll go and remove those. |
|
One other use case is that the man pages get generated for the readthedocs website (duh!). Any others? |
As a small addendum to this use case, I think it would be cool if we could have the RTD CI run on flux-core PRs like it does for the rfc and docs repos. I'm not sure if we just need |
|
@SteVwonder the travis.yaml file is super complex here... any idea where to start? maybe we can get together on friday to hack it out. :\ |
|
Question: which NAME section style do you prefer? Style 1 includes the other command that share the same man page text, style 2 just has the current man page name. The original style (included) is not possible with sphinx. |
What would the NAME section for the OR |
|
For both styles, the Basically the Sphinx generation puts “name - description” on this line, it just depends if we include the other related commands in the “description” text. |
I can't think of any. A finer point: the man pages should be generated with |
Awesome! In that case, my vote is for style 2 since it is cleaner. |
|
We should try to get this merged asap, so that we won't have to keep rebasing it as new or changed documentation lands in flux-core. Is there an option in the conversion process from adoc->rst to keep the line breaks from the source material? At least the files I spot checked, paragraphs end up all on one line. This might not be conducive to reviewing I'll try to take a look at getting sphinx run (when found) with |
Sorry for not posting a commit yesterday. I have a half-way baked It's pretty much a copy paste from the old
As long as we are OK requiring that the python that flux-core is configured against is the same python that has the sphinx module, I think we can treat it exactly like the other python dependencies. I was thinking we can use the I just gotta figure out the right automake syntax that supports "recursive" commands that can generate everything in one go, as opposed to compilers/asciidoc that can be run with one input file and produce one output file. |
This is the main issue I was anticipating. Thanks a lot for taking this on!! I didn't have time to look at it yet, but will happily test 😜 |
|
Just pushed two commits to a branch on my fork: https://github.com/SteVwonder/flux-core/commits/rst-docs They works for generating the manpages with |
gonsie
force-pushed
the
rst-docs
branch
3 times, most recently
from
24de6b1
to
5f61fac
Compare
|
@SteVwonder @grondo I think this branch is working! |
|
Thanks @gonsie for all the work on this PR! You are awesome! 🥇 I am getting an error when building the whole project. The All sorts of weirdness happens in diff --git a/doc/Makefile.am b/doc/Makefile.am
index 466747c1f..3bb863d1b 100644
--- a/doc/Makefile.am
+++ b/doc/Makefile.am
@@ -1,3 +1,5 @@
+.NOTPARALLEL:
+
SUBDIRS = . test
MAN_FILES = $(MAN1_FILES) $(MAN3_FILES) $(MAN5_FILES) $(MAN7_FILES)It looks like configure didn't find |
|
Looks like |
Yeah, that script processes comments in command man pages to determine if commands should be added to the generic [
{
"category": "core",
"command": "broker",
"description": "Invoke Flux comms message broker daemon"
},
{
"category": "core",
"command": "content",
"description": "Access instance content storage"
},
{
"category": "core",
"command": "cron",
"description": "Schedule tasks on timers and events"
},
{
"category": "core",
"command": "dmesg",
"description": "manipulate broker log ring buffer"
},
{
"category": "core",
"command": "env",
"description": "Print or run inside a Flux environment"
},
{
"category": "core",
"command": "event",
"description": "Send and receive Flux events"
},
{
"category": "core",
"command": "exec",
"description": "Execute processes across flux ranks"
},
{
"category": "core",
"command": "get,set,lsattr",
"description": "Access, modify, and list broker attributes"
},
{
"category": "core",
"command": "hwloc",
"description": "Control/query resource-hwloc service"
},
{
"category": "core",
"command": "jobs",
"description": "list jobs submitted to Flux"
},
{
"category": "core",
"command": "keygen",
"description": "generate keys for Flux security"
},
{
"category": "core",
"command": "kvs",
"description": "Flux key-value store utility"
},
{
"category": "core",
"command": "logger",
"description": "create a Flux log entry"
},
{
"category": "core",
"command": "mini",
"description": "Minimal Job Submission Tool"
},
{
"category": "core",
"command": "module",
"description": "manage Flux extension modules"
},
{
"category": "core",
"command": "ping",
"description": "measure round-trip latency to Flux services"
},
{
"category": "core",
"command": "proxy",
"description": "Create proxy environment for Flux instance"
},
{
"category": "core",
"command": "start",
"description": "bootstrap a local Flux instance"
},
{
"category": "core",
"command": "version",
"description": "Display flux version information"
}
] |
|
Can we just add the comments back to the source material? |
gonsie
force-pushed
the
rst-docs
branch
2 times, most recently
from
578510c
to
7ea6860
Compare
|
@grondo I added back the comments, but there is something buggy about Can you check out the last commit I pushed ( |
|
I ran into an issue testing this out in a VPATH build. (sphinx SRCDIR was listed as Finally, "quiet mode" make wasn't working because sphinx spews a lot of gak by default. And lastly, some manpages were mentioned twice and this generated automake warnings. This patch should fix all that up: diff --git a/doc/Makefile.am b/doc/Makefile.am
index 3bb863d1b..41fa5c2fa 100644
--- a/doc/Makefile.am
+++ b/doc/Makefile.am
@@ -196,7 +196,6 @@ MAN3_FILES_SECONDARY = \
man/flux_future_wait_for.3 \
man/flux_future_reset.3 \
man/flux_future_destroy.3 \
- man/flux_future_get.3 \
man/flux_future_fulfill.3 \
man/flux_future_fulfill_error.3 \
man/flux_future_fulfill_with.3 \
@@ -204,13 +203,11 @@ MAN3_FILES_SECONDARY = \
man/flux_future_aux_get.3 \
man/flux_future_set_flux.3 \
man/flux_future_get_flux.3 \
- man/flux_future_wait_all_create.3 \
man/flux_future_wait_any_create.3 \
man/flux_future_push.3 \
man/flux_future_first_child.3 \
man/flux_future_next_child.3 \
man/flux_future_get_child.3 \
- man/flux_future_and_then.3
man/flux_future_or_then.3 \
man/flux_future_continue.3 \
man/flux_future_continue_error.3 \
@@ -282,10 +279,19 @@ endif
SUFFIXES = .rst .1 .3 .5 .7
-$(MAN_FILES): $(RST_FILES)
- $(AM_V_GEN) $(PYTHON) -m sphinx -M man ./ ./
+sphinx_verbose = $(sphinx_verbose_$(V))
+sphinx_verbose_ = $(sphinx_verbose_$(AM_DEFAULT_VERBOSITY))
+sphinx_verbose_0 = @echo " SPHINX manpages";
+
+STDERR_DEVNULL = $(stderr_devnull_$(V))
+stderr_devnull_ = $(stderr_devnull_$(AM_DEFAULT_VERBOSITY))
+stderr_devnull_0 = >/dev/null 2>&1
+
+$(MAN_FILES): conf.py $(RST_FILES)
+ $(sphinx_verbose)$(PYTHON) -m sphinx -M man $(srcdir) ./ $(STDERR_DEVNULL)
EXTRA_DIST = \
+ conf.py \
$(RST_FILES) \
man1/NODESET.rst \
man3/JSON_PACK.rst \I changed the quiet output to just |
|
This pull request introduces 1 alert when merging b2f00a5 into 7953a1a - view on LGTM.com new alerts:
|
|
Ah, one more fix needed here. If sphinx is missing |
|
Ok, replaced a |
|
👏🏼👏🏼👏🏼thanks @grondo !! |
|
Thanks everybody! I'm excited to see how these look. |
|
Have to rebase again since #3068 went in. |
grondo
force-pushed
the
rst-docs
branch
2 times, most recently
from
53ed608
to
6f4a2e1
Compare
Update all manpage docs to reST from asciidoc. Convert all manpages, tools, autotools dependencies, etc. This is done all in one commit to avoid breaking the build with intermediate work. Summary of changes: - Auto-convert all adoc manpages to .rst and manually fix any issues - Move Author, Description, and NAME sections out of manpage source, this is handled in sphinx conf.py - By default, Sphinx wants to output all manpages in a single dir. To allow MANPATH in a build dir to work correctly, add a rule to move manpages to man1/, man3/, etc. after sphinx build to preserve compatibility with manpath. - Replace gen-cmdhelp.pl with a Python version, which uses sphinx's conf.py to assist in processing manpage sources. - Update autotools to look for sphinx deps instead of asciidoctor Co-authored-by: Stephen Herbein <stephen272@gmail.com> Co-authored-by: Mark A. Grondona <mark.grondona@gmail.com>
Prepare for conversion to sphinx for documentation. Remove ruby and asciidoctor, add sphinx and other deps.
|
In the process of pushing up the new docker images. Then this is officially ready for merge. |
Codecov Report
@@ Coverage Diff @@
## master #3033 +/- ##
=======================================
Coverage 81.19% 81.20%
=======================================
Files 285 285
Lines 44229 44229
=======================================
+ Hits 35912 35916 +4
+ Misses 8317 8313 -4
|
|
CI is now all green. @garlick gave an approval, @SteVwonder want to take a look too or shall we set MWP? |
|
Sorry @garlick, we cross posted. I'm ready to set MWP. We can fix up any fallout as we go along. |
|
MWP is fine with me. I agree, let's fix stuff in a follow up PR if anything is broken. Kinda confused why we aren't seeing the RTD's CI builder check thingy. I thought we added the webhook. I'll poke at that. |
|
Oh, yeah I meant to ask about that. I didn't see anything that pushes the built docs to RTD. (Though I didn't look carefully, was mainly concentrating on getting the conversion done so we don't conflict with future manpage updates.) |
|
MWP set! |
|
Thanks @gonsie and @SteVwonder!! 👏 |
|
YAY!!!!!!! 🥳 Per the RTD CI check, the webhook was working just fine. The issue was that the builder for PRs wasn't enabled in the Advanced Settings: https://docs.readthedocs.io/en/stable/guides/autobuild-docs-for-pull-requests.html. I flicked that on, hopefully it works now. 🤞 I also took the liberty of updating some RTD settings like where it looks for the |
|
Great, thanks! It is working https://flux-framework.readthedocs.io/projects/flux-core/en/latest/man1/flux-jobs.html One thing I notice is that on the left sidebar all the commands are listed in caps, e.g. |
|
Looks great! One other nice to have would be inline manpage references working as links, but probably for another time. Nice work everybody! |
Start of conversion for core documentation.
I’ll need some help integrating sphinx and such into the build system... all I’ve done thus far is remove the asciidoctor checks.