Page MenuHomePhabricator

[Flang] Fix release blocker issue #46931 related to documentation.
ClosedPublic

Authored by sameeranjoshi on Aug 6 2020, 12:55 PM.

Details

Summary

Support for sphinx builds.
This commit add a new flag -DLLVM_ENABLE_SPHINX=ON to cmake command to generate sphinx documentation, along with new targets docs-flang-html and docs-flang-man the latter target doesn't work currently, as the command line options are not yet documented for flang.

ninja docs-flang-html - generates sphinx documentation.
Generated release notes can be found in tools/flang/docs/html/ folder.

Diff Detail

Event Timeline

sameeranjoshi created this revision.Aug 6 2020, 12:55 PM
Herald added a project: Restricted Project. · View Herald TranscriptAug 6 2020, 12:55 PM
sameeranjoshi requested review of this revision.Aug 6 2020, 12:55 PM
sameeranjoshi retitled this revision from [Flang] Support for sphinx builds. Fix release blocker issue #46931 related to documentation. This commit add a new flag `-DLLVM_ENABLE_SPHINX=ON` to cmake command to generate sphinx documentation, along with new targets `docs-flang-html` and... to [Flang] Fix release blocker issue #46931 related to documentation. .Aug 6 2020, 1:00 PM
sameeranjoshi edited the summary of this revision. (Show Details)
sameeranjoshi added inline comments.Aug 6 2020, 1:02 PM
flang/docs/conf.py
44

Should this date be changed? If so what should be the right date ?

tskeith added a subscriber: tskeith.Aug 6 2020, 3:20 PM

Why do you need to copy files from llvm?

sameeranjoshi edited the summary of this revision. (Show Details)Aug 6 2020, 11:05 PM

Why do you need to copy files from llvm?

Sphinx comes with a set of built-in themes for webpages[1], I thought of sticking with the llvm style of release notes, so to add some custom theme not in built-in themes list you need respective files.
[1] https://www.sphinx-doc.org/en/master/usage/theming.html#builtin-themes

Thanks for adding all this infrastructure!
I don't see any CMake changes here though, where is the new flag added?

Why do you need to copy files from llvm?

Sphinx comes with a set of built-in themes for webpages[1], I thought of sticking with the llvm style of release notes, so to add some custom theme not in built-in themes list you need respective files.
[1] https://www.sphinx-doc.org/en/master/usage/theming.html#builtin-themes

Sticking to llvm style sounds good. But as soon as it is changed in llvm we won't match the llvm style any more. Does clang have copies like this too?

Sticking to llvm style sounds good. But as soon as it is changed in llvm we won't match the llvm style any more. Does clang have copies like this too?

I believe clang uses some cmake infrastructure to get doxygen and sphinx to pick up the theming stuff from LLVM. I at least can't see copies in clang.

Thanks for adding all this infrastructure!
I don't see any CMake changes here though, where is the new flag added?

It was added with doxygen commits

Sticking to llvm style sounds good. But as soon as it is changed in llvm we won't match the llvm style any more. Does clang have copies like this too?

I believe clang uses some cmake infrastructure to get doxygen and sphinx to pick up the theming stuff from LLVM. I at least can't see copies in clang.

There is a configuration variable html_theme in each sub-project supporting sphinx, I tried to grep "html_theme" -r here's the summary.
clang, libcxx, clang-tools-extra, libunwind - Use built-in theme haiku
llvm, flang, lld - Use custom llvm based theme llvm-theme
lldb - Uses alabaster as one of the built-in themes.

Checking the git blame logs for these files in llvm and in lld shows they weren't modified since 2012, so they seem to be pretty consistent since then.

Oh I see, I didn't realise clang uses the default theme not the LLVM one! If lld also copies these files I think it is fine for us to do so as well.

Regarding the release note contents. My original review proposing these did not get any strong objections so I propose that once we are all happy with the code that this initial version of the release notes be committed alongside. I will make a follow up review with any further edits to it before release.

Alternative plan might be to add my master placeholder release note from https://reviews.llvm.org/D84867 instead of the version specific to the LLVM11 branch from https://reviews.llvm.org/D84864

richard.barton.arm requested changes to this revision.Aug 10 2020, 10:22 AM

These changes build for me and I get a .html version of the flang release notes in the docs directory in the build. It is a shame there is not more content there and I suppose we'll need to convert the docs in /documentation over to .rst format to get them included in this set.

As pointed out in the developer call today. There does not seem to be much objection to this change, and it is successfully building so I am happy to approve it. Please wait a day or so to give a chance for others to intervene if they want further changes.

flang/docs/conf.py
44

I think this should be 2017.

This revision now requires changes to proceed.Aug 10 2020, 10:22 AM
This revision is now accepted and ready to land.Aug 10 2020, 10:22 AM

Updated copyright year.

sameeranjoshi marked an inline comment as done.Aug 11 2020, 4:54 AM
hans added a comment.Aug 20 2020, 7:53 AM

Merged to 11.x in b0b18ec9e8a7e7c7ca6e01d1a6f636622fa59941. Thanks!

It would probably make sense to have an index.rst file too, to generate index.html.