Horton

Re: meson documentation build open issues

Andres Freund <andres@anarazel.de>

View thread View on postgr.es
From: Andres Freund <andres@anarazel.de>
To: Peter Eisentraut <peter@eisentraut.org>
Cc: Christoph Berg <myon@debian.org>, Andrew Dunstan <andrew@dunslane.net>, Tom Lane <tgl@sss.pgh.pa.us>, pgsql-hackers <pgsql-hackers@postgresql.org>
Date: 2023-11-15T00:22:31Z
Lists: pgsql-hackers 
Hi,

On 2023-11-14 21:16:13 +0100, Peter Eisentraut wrote:
> Some comments on your patches:
> 
> v2-0001-meson-Change-default-of-selinux-feature-option-to.patch
> 
> Ok
> 
> v2-0002-docs-Document-with-selinux-Dselinux-options-centr.patch
> 
> Ok, but "selinux" should be "SELinux" when referring to the product.

Will apply with that fix.


> v2-0003-meson-docs-Add-doc-html-man-targets.patch
> 
> We have make targets "html" and "man", so I suggest we make the meson
> targets the same.

Hm, ok.


> v2-0004-meson-Add-world-target.patch
> 
> AFAICT, this world target doesn't include the man target.  (Again, this
> would all work better if we added "man" to "docs".)

I agree with that sentiment - I only moved to the current arrangement after
Tom argued forcefully against building both.

The situation in the make world is weird:
"make docs" in the toplevel builds both, because it's defined as

docs:
	$(MAKE) -C doc all

Buf if you "make -C doc/src/sgml" (or are in doc/src/sgml), we only build
html, as the default target is explicitly just html:

# Make "html" the default target, since that is what most people tend
# to want to use.
html:


There's no real way of making the recursive-make and non-recursive ninja
coherent. There's no equivalent to default target in a sudirectory with ninja
(or non-recursive make).


> v2-0005-docs-meson-Add-documentation-for-important-build-.patch
> 
> It's nice to document this, but it's weird that we only document the meson
> targets, not the make targets.

I think it'd have been good if we had documented the important targets with
make. But I don't think documenting them as a prerequisite to documenting the
meson targets makes much sense.


> v2-0006-meson-Add-help-target-build-docs-from-a-common-so.patch
> 
> Here also, making this consistent and uniform with make would be useful.

What precisely are you referring to here? Also adding a help target? Or just
consistency between what the "docs" target does?


> v2-0007-meson-Add-Dpkglibdir-option.patch
> 
> Normally, the pkgFOOdir variables are just FOOdir plus package name.  I
> don't feel comfortable allowing those to be separately set.  We don't allow
> that with configure; this just arose from a Debian patch.

Right - but Debian's desire seems quite sensible. The need to have multiple
postgres versions installed in parallel is quite widespread.


> The description "location to dynamically loadable modules" is too narrow.
> Consider for example, another proposed patch, where we are doing some
> preprocessing on postgres.bki at build time.  Since that makes postgres.bki
> platform dependent, it should really be moved from share (datadir in
> configure parlance) to pkglibdir.

I think I cannot be faulted for documenting the current use of the directory
:).

Separately, I'm not really convinced that moving some build time values into
postgres.bki is useful, but that's a matter for a different thread.


> So then we have things in there that are not loadable modules.  I don't know
> how that affects Debian packaging, but this patch might not be the right
> one.

I'm not really seeing why that'd affect pkglibdir being adjustable, besides
needing to tweak the description of pkglibdir?


> I suggest we leave this patch for a separate discussion.

Fair enough.

Thanks for the review,

Andres Freund

Commits

  1. docs: meson: Change what 'docs' target builds

  2. meson: Add 'world' target

  3. meson: Document build targets, add 'help' target

  4. meson: docs: Add {html,man} targets, rename install-doc-*

  5. docs: Document --with-selinux/-Dselinux options centrally

  6. meson: Change default of 'selinux' feature option to auto

  7. meson: Fix doc installation path computation

  8. meson: Install missing example files

  9. meson: docs: Install all manpages, not just ones in man1

  10. meson: docs: Allow configuring simple/website style

  11. docs: html: load stylesheet via custom.css.source

  12. docs: html: copy images to output as part of xslt build

  13. meson: docs: Preparatory cleanups

  14. docs: Remove support for 'htmlhelp' format

  15. meson: add install-{docs,doc-html,doc-man} targets

  16. meson: add install-{quiet, world} targets

  17. meson: make install_test_files more generic, rename to install_files

  18. meson: rename html_help target to htmlhelp