-
Notifications
You must be signed in to change notification settings - Fork 15.9k
[openmp] Build doxygen in bootstrapping builds #178298
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
Conversation
boomanaiden154
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM, but might want to let someone more knowledgeable about CMake also review.
Are there any other runtimes that also publish doxygen docs that we would want to do this for?
| if (LLVM_ENABLE_DOXYGEN) | ||
| list(APPEND extra_targets "doxygen-openmp") | ||
| if (TARGET doxygen) | ||
| add_dependencies(doxygen doxygen-openmp) | ||
| endif () | ||
| endif () |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'd like to find a way to generalize this because hardcoding this for every runtime doesn't scale. Ideally, every runtime would provide doxygen-${runtime_name} target similarly to install-${runtime_name} above. That doesn't have to happen in this PR though, but I'd appreciate if you could at least leave a TODO comment here.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
done
openmp is the only one. While there is a The proposed solution to add a |
|
LLVM Buildbot has detected a new failure on builder Full details are available at: https://lab.llvm.org/buildbot/#/builders/27/builds/22304 Here is the relevant piece of the build log for the reference |
When
LLVM_ENABLE_DOXYGEN=ON, forward thedoxygen-openmpbuild target from the nested (default target) runtimes build. WhenLLVM_BUILD_DOCS=ON, also triggerdoxygen-buildwithninja doxygen.LLVM_INCLUDE_DOCS=ONis required in the runtimes build, which is the default.This is required to update the OpenMP doxygen documentation at https://openmp.llvm.org/doxygen by the publish-doxygen-docs buidbot, discussed here: llvm/llvm-zorg#716 (review)