OpenNTI / sphinxcontrib-programoutput

Sphinx extension for capturing program output
BSD 3-Clause "New" or "Revised" License
38 stars 18 forks source link

sphinxcontrib.programoutput extension does not declare if it is safe for parallel reading #25

Closed adamjstewart closed 7 years ago

adamjstewart commented 7 years ago

Hey, glad to see this package being revived! We use programoutput for our Spack documentation and it's very useful.

When I build our documentation in parallel, I see the following warning message:

WARNING: the sphinxcontrib.programoutput extension does not declare if it is safe for parallel reading, assuming it isn't - please ask the extension author to check and make it explicit
WARNING: doing serial read

Is there any way to get programoutput to work in parallel? If not, can we explicitly declare that it doesn't support parallel reading to remove this warning message?

@svenevs may be interested in this conversation.

jamadden commented 7 years ago

Interesting, I don't recall having seen that message before. Did it start very recently, with a newer version of Sphinx, perhaps?

adamjstewart commented 7 years ago

Oh no, this has been happening for years. It only occurs when you build in parallel:

sphinx-build -j 4 -b html . _build/html

Here, -j 4 tells Sphinx to build in parallel with 4 jobs. This greatly speeds up large documentation builds.

jamadden commented 7 years ago

Oh no, this has been happening for years. It only occurs when you build in parallel:

Oh, well, I guess that makes sense then 😄

It's probably going to be awhile before I have a chance to dig into this, but if you get a chance before then, I could probably review a PR.

svenevs commented 7 years ago

@jamadden whatever is inheriting from sphinx.application.Sphinx just needs to state explicitly whether or not it can do parallel. To make it go away, just do parallel=0. I haven't looked into this for your code, but somehow this default is getting ignored? TBH I can't even find any docs on it, but it's here and some discussion here for a little more info, primarily linking to show this commit

As to whether or not your application actually is threadsafe...hehehe. Welllll if you just want to squelch the warning say it is not :upside_down_face:

jamadden commented 7 years ago

I think we should generally be parallel read safe. I don't have time to do extensive testing right now, but if you want to try #26 that might take care of it.

adamjstewart commented 7 years ago

I tried out #26 and got a lot of errors. But I think those errors may actually be our fault, not programoutput. The errors mostly involve links to non-existing sections of the documentation. I think what is going on is that each page of the documentation is being built in a random order, and depending on the order, some links don't exist yet. So we may need to tell our documentation to build in serial. In fact, programoutput forcing our documentation to build in serial may be the only reason we haven't discovered this problem already 😆

adamjstewart commented 7 years ago

Closing this as #26 has now been merged. Thanks for your help @jamadden!