Re: Autogenerate some wait events code and documentation

On Thu, May 18, 2023 at 01:36:30PM +0900, Michael Paquier wrote:
> The order looks fine seen from here, thanks!

Now that v17 is open for business, I have looked again at this patch.

perlcritic is formulating three complaints:
./src/backend/utils/activity/generate-waiteventtypes.pl: Loop iterator
is not lexical at line 99, column 1. See page 108 of PBP.
([Variables::RequireLexicalLoopIterators] Severity: 5)
./src/backend/utils/activity/generate-waiteventtypes.pl: Loop iterator
is not lexical at line 126, column 1. See page 108 of PBP.
([Variables::RequireLexicalLoopIterators] Severity: 5)
./src/backend/utils/activity/generate-waiteventtypes.pl: Loop iterator
is not lexical at line 181, column 1. See page 108 of PBP.
([Variables::RequireLexicalLoopIterators] Severity: 5)

These are caused by three foreach loops, where perl wants to use a
local declaration for the iterators.

The indentation was a bit off, as well, perltidy v20230309 has
reported a few diffs. Not a big deal.

src/common/meson.build includes the following comment:
# For the server build of pgcommon, depend on lwlocknames_h, because at least
# cryptohash_openssl.c, hmac_openssl.c depend on it. That's arguably a
# layering violation, but ...

The thing is that controldata_utils.c has a dependency to wait events
so we should add wait_event_types_h to 'sources'.

The names chosen, as of wait_event_types.h, pgstat_wait_event.c,
waiteventnames.txt and generate-wait_event_types.pl are inconsistent,
comparing them for instance with the lwlock parts. I have renamed
these a bit, with more underscores.

Building the documentation in a meson/ninja build can be done with the
following command run from the root of the build directory:
ninja alldocs

However this command fails with v10. The step that fails is:
[6/14] Generating doc/src/sgml/postgres-full.xml with a custom command

It seems to me that the correct thing to do is to add --outdir
@OUTDIR@ to the command? However, I do see a few problems even after
that:
- The C and H files are still generated in doc/src/sgml/, which is
useless.
- The SGML file wait_event_types.sgml in doc/src/sgml/ seems to be
empty, still to my surprise the HTML part was created correctly.
- The SGML file is not needed for the C code.

I think that we should add some options to the perl script to be more
selective with the files generated. How about having two options
called --docs and --code to select one or the other, then limit what
gets generated in each path? I guess that it would be cleaner if we
error in the case where both options are defined, and just use some
gotos to redirect to each foreach loop to limit extra indentations in
the script. This would avoid the need to remove the C and H files
from the docs, additionally, which is what the Makefile in doc/ does.

I have fixed all the issues I've found in v11 attached, except for the
last one (I have added the OUTDIR trick for reference, but that's
incorrect and incomplete). Could you look at this part?
--
Michael