Skip to content
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

Use meson to generate docs (html + pdf) #15169

Open
wants to merge 9 commits into
base: master
Choose a base branch
from

Conversation

omoerbeek
Copy link
Member

@omoerbeek omoerbeek commented Feb 17, 2025

Short description

Same script used for rec, auth and dnsdist docs. All meson.build file gain a few new targets: html_docs, html-docs.tar.bz2 and product-name.pdf and a do all called all-docs.

To test run meson compile -C builddir *target*.

Note that generating a pdf needs latex. The meson script does not check for that atm.

TODO:

  • generating doc sources(by dnsdist-rust-lib/dnsdist-settings-documentation-generator.py) for dnsdist (see Makefile.am in dnsdist-rust-lib) these file are actually in git, so no need to generate them

Checklist

I have:

  • read the CONTRIBUTING.md document
  • compiled this code
  • tested this code
  • included documentation (including possible behaviour changes)
  • documented the code
  • added or modified regression test(s)
  • added or modified unit test(s)
  • checked that this code was merged to master

@omoerbeek omoerbeek changed the title Rec meson gen docs Use meson to generate docs (html + pdf) Feb 17, 2025
@omoerbeek omoerbeek force-pushed the rec-meson-gen-docs branch 2 times, most recently from b2186d3 to 01ee2c5 Compare February 17, 2025 14:50
@coveralls
Copy link

coveralls commented Feb 17, 2025

Pull Request Test Coverage Report for Build 13394452463

Details

  • 0 of 0 changed or added relevant lines in 0 files are covered.
  • 53 unchanged lines in 12 files lost coverage.
  • Overall coverage decreased (-0.002%) to 64.497%

Files with Coverage Reduction New Missed Lines %
modules/gpgsqlbackend/gpgsqlbackend.cc 1 88.62%
pdns/distributor.hh 2 51.86%
pdns/dnsdistdist/dnsdist-crypto.cc 2 75.72%
pdns/recursordist/rec-tcpout.cc 2 63.49%
pdns/ws-auth.cc 2 80.79%
ext/yahttp/yahttp/url.hpp 3 54.42%
pdns/opensslsigners.cc 3 61.2%
pdns/recursordist/recpacketcache.hh 3 89.55%
pdns/misc.cc 4 62.84%
pdns/recursordist/syncres.cc 6 80.17%
Totals Coverage Status
Change from base Build 13374437384: -0.002%
Covered Lines: 127623
Relevant Lines: 166873

💛 - Coveralls

The Meson docs are not a fan of this method, but maintaining that
list by hand is too much work and error prone. This does mean that
adding existing .rst file to the docs will not be picked up by the
dependency checking until you re-setup your build dir.
@omoerbeek omoerbeek marked this pull request as ready for review February 17, 2025 15:31
meson.build Outdated
Comment on lines 1128 to 1129
find_rst_files = run_command(['find', docs_dir, '-name', '*.rst'])
rst_files = find_rst_files.stdout().strip().split('\n')
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This won't be updated when new files are added. The QEMU project faced this challenge already, and wrote a neat trick to solve it:

https://gitlab.com/qemu-project/qemu/-/blob/master/docs/sphinx/depfile.py

This allows you to use depfile: ... and have sphinx report what files it used and which should trigger further rebuilds.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah, I mentioned the drawback in the commit message. I'll look into it.

Copy link
Contributor

@eli-schwartz eli-schwartz Feb 18, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, Meson isn't a fan of this run_command method... because it's better done by depfiles any time you don't explicitly need the input files as command line arguments! :)

(When you do need them as command-line arguments, for example with executable() sources, I tend to recommend running a generator script as a pre-commit hook, then using fs.read('sources.list').strip().split('\n') instead. fs.read will produce reconfigure dependencies, much like an automake include.)

Copy link
Contributor

@eli-schwartz eli-schwartz Feb 18, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actually, the depfile (sphinx.d) is removed by nija after generating
it as shown by a trace. I do not know why, but touching an .rts
file has the correct behaviour. So I suspoect it is an optimization.

--> ninja -d list

debugging modes:
[...]
  keepdepfile  don't delete depfiles after they're read by ninja

We use depfiles for all *.o outputs as well. Ninja reads them in after the command finishes for build nodes that declare they provide depfiles, and compiles them into a binary database (.ninja_deps) for, as you guessed, optimization purposes. No need to read tons of tiny files in on every startup. :)

Hence also why keeping the file around is one of the options exposed in the debugging modes. Though, I don't know why it is necessary to delete them after reading them, but maybe the ninja authors figured the file is useless after reading it, so might as well compact the build directory size a tiny bit.

omoerbeek added a commit to omoerbeek/pdns that referenced this pull request Feb 18, 2025
… file, as suggested by @eli-schwart in PowerDNS#15169 (comment)

Actually, the depfile (sphinx.d) is removed by nija after generating
it as shown by a trace.  I do not know why, but touching an .rts
file has the correct behaviour. So I suspoect it is an optimization.
… file, as suggested by @eli-schwart in PowerDNS#15169 (comment)

Actually, the depfile (sphinx.d) is removed by ninja after generating
it as shown by a trace.  I do not know why, but touching an .rst
file has the correct behaviour. So I suspect it is an optimization.
Co-authored-by: Josh Soref <2119212+jsoref@users.noreply.github.com>
@omoerbeek omoerbeek requested review from rgacogne and Habbie February 24, 2025 08:24
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants