Older review comments at: https://github.com/flang-compiler/f18/pull/1065
Details
Diff Detail
Event Timeline
This looks good to me, thanks! If you get approval from @sscalpone or @richard.barton.arm as well I think this is ready to land.
Thanks @david, how do we add this flags to the buildbots of LLVM?
I had idea of .drone.ci file, how about phabricator now?
I have no idea :) the precommit on phabricator is very new and I haven't looked in to it at all. Most CI is focussed on BuildBots which run after commit, and those just have whatever flags the person hosting it have set. I wouldn't worry about it too much at the moment, we need to discuss what is going on with the folks that set them up.
Above doxygen patch seems to fail for me for below flags
cmake ../llvm
-DLLVM_ENABLE_DOXYGEN=ON \
-DLLVM_BUILD_DOCS=OFF \
-DFLANG_INCLUDE_DOCS=OFF \
ninja install
As it tries to copy <path to build>/docs/html to <path to install prefix>/docs the same error for out-of-tree reported (here) which I tried to fix for out-of-tree builds (here)
Now that we are in tree, things seem to have dependency on mlir and llvm as well.
llvm-project/mlir as well shows the same error.
When adding the dependency (here) to llvm and mlir docs/CMakeLists.txt seems to fix error.
@sscalpone is that the expected way how the doxygen is tested in other llvm umbrella projects or am I testing it in wrong way?
@jdoerfert does clang and mlir also have the similar issue, which needs to be fixed, as adding a fix in these projects seems to solve issue in flang ?
I'm not really sure if this combination of flags makes sense anyway, what's the expected behaviour when using these flags? I can't really tell intuitively
Expected behavior should not build nor install any documentation and documentation targets(doxygen-flang) should not be visible.
Whereas adding LLVM_INCLUDE_DOCS=OFF and MLIR_INCLUDE_DOCS=OFF seems to work fine.
cmake ../llvm
-DLLVM_ENABLE_DOXYGEN=ON \
-DLLVM_BUILD_DOCS=OFF \
-DFLANG_INCLUDE_DOCS=OFF
-DLLVM_INCLUDE_DOCS=OFF \
-DMLIR_INCLUDE_DOCS=OFF \
But does anyone have an idea on the appropriate flags for generating doxygen based documentation, as the number of permutations could be huge to test based on llvm and mlir flags?
Are you sure? I would have thought LLVM_ENABLE_DOXYGEN does expose the doxygen-flang target. But again I don't really have much of an idea.
The CMakeLists.txt file has
if (FLANG_INCLUDE_DOCS) add_subdirectory(docs) endif()
When
FLANG_INCLUDE_DOCS=OFF
IIUC the working of CMake I think the docs/ sub directory won't be added hence no docs/CMakeLists.txt would be present.
The patch is updated with README and tested enough to work based on the recommended way of generating documentation in LLVM.
https://reviews.llvm.org/rG807fe05d3531cae47d0c770b2d919f861bfe9235
Above addresses and closes the review.