Doxygen error check with cmake build

[nila]

Now, with commit 140c81a50a, doxygen documentation can be build with cmake.

To limit doxygen errors in future commits, it would be good to implement a build time error check for cmake, as has been done for autotools.

I have made a proposal for how this can be done in ​https://git.osgeo.org/gitea/geos/geos/pulls/95. It is pure cmake language error check, thus platform independent. Unfortunately, I have not the means to test on linux or windows, so it needs testing.

[dbaston]

Thank you! I had started along these lines but hadn't finished.

To ignore "not documented" errors I think we should set the appropriate flag in Doxyfile rather than grepping the output.

It would be nice to understand the source of the "invalid tag" errors too ; maybe we can avoid those by specifying a required Doxygen verison.

These changes would allow a simpler solution of just having Doxygen stop on error instead of parsing the log files.

Opening a pull request at GitHub will test the changes against roughly 40 platforms; however, documentation is only built on Linux. I will look at enabling Doxygen for OS X / Windows builds.

[nila]

I'm glad you appreciate it :)

"Not documented" warnings are *very* numerous, in fact I ticked that Doxygen flag fixing less common warnings earlier this year.

The invalid tags comes from updating Doxyfile from 1.2.15 (!) to 1.8.14 with ​https://git.osgeo.org/gitea/geos/geos/pulls/81. There are lags of varying degrees on different platforms in regard to Doxygen version, therefore I believe it would be difficult to reach an optimal solution. (The official ​https://geos.osgeo.org/doxygen/ documentation is generated with v1.8.8 from 2014). I don't think it has a major impact on the result (although I did notice unexpected result on the official documentation compared with what I had locally, but that was not necessarily due to invalid tags).

The reason @strk implemented the check (see: geos/Makefile.am:doxygen-checked) actively running (successfully!) on Drone, the way he did, I can only speculate. Above mentioned pull request mirrors its implementation.

I'm aware of the GitHub possibilities, but I didn't want – at this stage – to push for changes in CI build-files in this PR/commit (the implementation introduced a new build target make doxygen-checked (again autotools mirrored), that's why make docs would not do).

Open for suggestions for how to proceed...

[strk]

I'm ok to ignore 'not documented' errors via Doxygen flags, if supported by Doxygen 1.2.15. I used the grep because it was guaranteed to work w/out having to do too much research... Error out on warning would only be possible when all warnings are fixed, I was going there incrementally

[nila]

As I suspected @strk, but didn't want to put words in your mouth. And I agree. Better to keep all warnings in logfile, what's hidden is forgotten. I made a small change in the error check cmake-script, which states in build log how many warnings had been issued: e.g.

-- Doxygen issued 1702 warning(s), see /home/travis/build/libgeos/geos/_build/doc/doxygen.log

regardless if the build succeeds or not.

I've tested reverting the Doxygen.in file to version 1.8.8, and setting WARN_IF_UNDOCUMENTED to NO. Building with 1.8.15 didn't issue any warnings, so this could be a way to go, but in the future obsolete tags might appear as well, so... For now, I think we should decide what warnings we can allow to be ignored.

@dbaston I opened a PR on github as well ​https://github.com/libgeos/geos/pull/224, with intentional doxygen errors and the doxygen-checked target activated on travis, and the build failed as expected:

Built target docs
Scanning dependencies of target doxygen-checked
-- Doxygen issued 1702 warning(s), see /home/travis/build/libgeos/geos/_build/doc/doxygen.log
CMake Error at check_doxygen_errors.cmake:42 (message):
  /home/travis/build/libgeos/geos/include/geos/noding/SegmentNode.h:70:
  warning: argument 'THISISCOMPLETELYWRONGTOO' of command @param is not found
  in the argument list of geos::noding::SegmentNode::SegmentNode(const
  NodedSegmentString &ss, const geom::Coordinate &nCoord, size_t
  nSegmentIndex, int nSegmentOctant)

  /home/travis/build/libgeos/geos/include/geos/planargraph/Edge.h:88:
  warning: argument 'THISISCOMPLETELYWRONG' of command @param is not found in
  the argument list of geos::planargraph::Edge::Edge(DirectedEdge *de0,
  DirectedEdge *de1)

    parameter 'de0'
    parameter 'de1'


doc/CMakeFiles/doxygen-checked.dir/build.make:57: recipe for target 'doc/CMakeFiles/doxygen-checked' failed
make[3]: *** [doc/CMakeFiles/doxygen-checked] Error 1
CMakeFiles/Makefile2:1888: recipe for target 'doc/CMakeFiles/doxygen-checked.dir/all' failed
make[2]: *** [doc/CMakeFiles/doxygen-checked.dir/all] Error 2
CMakeFiles/Makefile2:1895: recipe for target 'doc/CMakeFiles/doxygen-checked.dir/rule' failed
make[1]: *** [doc/CMakeFiles/doxygen-checked.dir/rule] Error 2
Makefile:795: recipe for target 'doxygen-checked' failed
make: *** [doxygen-checked] Error 2
The command "./tools/ci/script.sh" exited with 2.

Now, it should be tested on Windows too (I've tested on macOS). @dbaston, could you please help out with this?

[nila]

As ​suggested by @dbaston I made the doxygen error check a cmake test (instead of a new build target). I have now cleaned up commit history and removed intentional test-bugs. Build test on travis works fine (although at the moment doxygen errors cause failure, something ​PR#97 probably will solve).

As the test is only executed with doxygen installed, it will not affect present Windows or macOS CI builds. It might not be too great a risk to go for this before further testing and deal with eventual problems as they present themselves?!

As I see it, the PR is now ready for merge.

[dbaston]

Merged