build: add doc target

[Tachi107]

Here are the two commit messages:

1. This patch adds a doc target generating Doxygen documentation similarly to build: add doc target zycore-c#51, but with some extra bits: if doxygen-awesome.css is found on the host system, CMake will automatically use it to generate themed documentation.

   Using doxygen_add_docs() requires setting the various Doxygen options in CMakeLists.txt, so there's currently a bit of duplication as the same options are also contained in Doxyfile and Doxyfile-themed. I couldn't remove the two standalone files as they are still required by the doc.yml CI/CD pipeline, as zydoc runs Doxygen itself; as a solution, zydoc could add support for a --doxygen-generated <path> flag that tells it to use already generated Doxygen documentation instead of generating its own.

2. This patch tweaks the Makefile a bit to make it use the new doc target.

   A new configure target was added, that simply runs cmake -B build_dir, so that make doc doesn't require building the full project.

   I've also made a few minor changes like using cmake --build and cmake --install instead of plain make, as they lead to more concise commands and also make switching build backend easier (e.g. you simply need to add -G Ninja to the configure target, without having to replace all the make calls to ninja ones).

   Another fix was changing &> /dev/null to > /dev/null, as they broke the if statement on my system, and also because POSIX mandates that command -v should print to stdout on success, and shouldn't print anything on error (i.e. no stderr is involved).

[athre0z]

Hi and thanks for the PR!

There's a few problems that I see with letting CMake generate this:

• The documentation is now built every time I type make, which I find annoying because Doxygen spams a ton of stuff into my terminal, making it hard to see the actual relevant warnings from the C compiler
  • This can probably easily be fixed by introducing a default-OFF ZYDIS_BUILD_DOC option and/or cranking down Doxygen's verbosity
  • From the top of my head, I think my preference would be making this option default-ON and silencing Doxygen to only print errors. Documentation generation is sufficiently fast to not be annoying during development.
• The configuration is now duplicated in two places, and without the duplication there is no obvious way forward to retain the current zydoc behavior: zydoc re-builds the documentation for every Zydis version on each build. It takes the Doxyfile from master and uses that to build documentation for all versions. The idea here was to have consistent documentation theming and configs across all versions. Admittedly, this approach has its drawbacks -- e.g. breaking Google-indexed links if Doxygen changes HTML generation -- but I'm also not looking forward to spending time on re-engineering this.

[Tachi107]

• The documentation is now built every time I type make, which I find annoying because Doxygen spams a ton of stuff into my terminal, making it hard to see the actual relevant warnings from the C compiler

  • This can probably easily be fixed by introducing a default-OFF ZYDIS_BUILD_DOC option and/or cranking down Doxygen's verbosity

That's true, and I find it quite annoying too. I'd personally make Doxygen quieter instead of adding a new build option, as I don't see why you wouldn't want to generate documentation if Doxygen is found on the host, and also because if you really want to you can already tell CMake to treat Doxygen as not found with -DCMAKE_DISABLE_FIND_PACKAGE_Doxyen=true. Verbosity can be reduced with Doxygen's QUIET option.

• The configuration is now duplicated in two places, and without the duplication there is no obvious way forward to retain the current zydoc behavior: zydoc re-builds the documentation for every Zydis version on each build. It takes the Doxyfile from master and uses that to build documentation for all versions. The idea here was to have consistent documentation theming and configs across all versions. Admittedly, this approach has its drawbacks -- e.g. breaking Google-indexed links if Doxygen changes HTML generation -- but I'm also not looking forward to spending time on re-engineering this.

When looking at zydoc I didn't realize that it builds documentation for all branches, so my suggestion to tell zydoc to use already generated documentation is not actionable :/

I could write a little CMake script to tell CMake to import the options from the Doxyfile; does this sound reasonable to you?

Edit: done, the options are now read from the Doxyfile; here's the updated commit message: Using doxygen_add_docs() requires setting the various Doxygen options in CMakeLists.txt, so to avoid duplicating things in different places I wrote a bit of CMake code that parses the Doxyfile and sets the various options as CMake variables prefixed by DOXYGEN_. The "parser" is really basic, and does not handle values that span over multiple lines.

[athre0z]

Thanks -- that solves all major pain points! :)

There appears to be an issue with the README.md being picked up, though. The index.html page is blank for me when documentation is built on your branch.

[athre0z]

Nit: this temporary variable DOXYFILE is upper-cased whereas later locals are lower-cased -- is there a reason for this?

[Tachi107]

Il giorno dom 2 ott 2022 alle 11:21:38 -07:00:00, Joel Höner ***@***.***> ha scritto:

There appears to be an issue with the README.md being picked up, though. The index.html page is blank for me when documentation is built on your branch.

Thanks for noticing this, I somehow forgot to check that the parser actually worked 😅️ The issue was caused by the fact that option_name contained trailing whitespace, and is now fixed. As for the reason why option, option_name and option_value are lowercase that's because they are only used as temporary variables inside the loop, and are not result variables (unlike DOC_PATHS, for examples). This seems to be a CMake convention I've seen a few times in Find Modules.

…

-- OpenPGP key: 66DE F152 8299 0C21 99EF A801 A8A1 28A8 AB1C EE49

[athre0z]

LGTM, thanks! :)

[Tachi107]

Thanks to you for being so collaborative :D