Re: [PATCH 1/9] Add a target to check example programs

[Stephen Kitt <steve@sk2.org>]

[-- Attachment #1: Type: text/plain, Size: 4875 bytes --]

Hi Alex,

On Sat, 8 Jan 2022 02:15:47 +0100, "Alejandro Colomar (man-pages)"
<alx.manpages@gmail.com> wrote:
> On 1/7/22 17:46, Stephen Kitt wrote:
> > This is a first step to automating example program checks. It extracts
> > source snippets introduced by "Program source", assuming it's C, and
> > attempts to compile it.
> > 
> > For now, only man pages with a single "Program source" entry are
> > processed. The compiled code isn't linked to avoid having to handle
> > library requirements (e.g. -ldl).
> > 
> > Signed-off-by: Stephen Kitt <steve@sk2.org>  
> 
> I like the concept of this patch.
> 
> However, a few things:
> 
> - I prefer a script in ./scripts/,
>    which can be called from the Makefile.
>    I'd like to keep the Makefile simple
>    (it's already quite complex to add more stuff to it).
> 
> - I'd like to use make(1) properly,
>    and only test programs incrementally,
>    so if a page has already been tested and it hasn't been modified,
>    it should not be retested.
>    That part should go in the Makefile, not the test itself.
>    For that, I'd chose some arbitrary dirname
>    (<./tests/example_programs/> sounds like a good candidate),
>    and touch dummy files there when a test is performed:
> 
> 
> builddir := $(CURDIR)/build
> TESTS_example_programs := $(patsubst 
> $(MANDIR)/%,$(builddir)/%.example-programs.touch,$(MANPAGES))
> 
> $(TESTS_example_programs): $(builddir)/%.example-programs.touch: \
>                                        $(MANDIR)/% \
>                                        Makefile \
>  
> $(CURDIR)/scripts/check_example_programs.sh \
>                                        | $$@D)/.
> 	$(info TEST example programs:	$@)
> 	$(CURDIR)/scripts/check_example_programs $@
> 	touch $@
> 
> 
> Something like the above would be nice.
> That would also remove the call to mktemp(1).

Yes, that’s along the lines of what I was thinking too; in particular,
something like that would be necessary to handle man pages with multiple
example files (e.g. unix.7).

The main purpose of this patch was to get the conversation started and to
show how I’d identified the problems I was fixing in the rest of the series.
I wanted to have something simple to get going, just to measure how likely it
was that there actually were problems in the example programs before
embarking on a more complex process ;-).

> - This expects pages to have 'Program source' just before the source.
>    Few pages do have that currently.
>    Do you have plans to standardize some convention in the man pages?
>    I'd be happy to see that.
>    I think a good approach would be to use C comments,
>    one line before the code, and one line after the code,
>    so that there's no doubts about the limits of the code
>    (we should decide the format of the comment).
>    The comment should be something not too noisy, but very reliable.
> 
>    I think I'd also first restrict to the EXAMPLES sections,
>    even before reading that hypothetical comment (or whatever we put),
>    to avoid mistakes.

At first I’d tried going with troff comments in the man page sources, but
that doesn’t work because we rely on the troff *output* being correct C, not
the troff input. So we need something that can be processed from the output,
and “Program source” was available for a first stab.

Ultimately though, as you say, we’d probably need C comments to start and
finish the code. That would fit in nicely with a Make-based approach: we
could have one target to generate the “dependencies” (C files generated from
man pages), then include that — that way we’d only update C files when the
corresponding man page changes.

> - Logs should go to stdout/stderr,
>    as in any other standard Unix command,
>    so just let the compiler print wherever it wants to print,
>    and let the user redirect them to wherever the user wants to too.
> 
>    I know there was the groff-warnings test.  But I don't like it.
>    It's there because it predates me,
>    and I yet have to understand how and if it works,
>    and then I'll rewrite it properly.

And I was basing my code on the groff-warnings test ;-). From running a
number of variants, what I liked about this is that it provides output
(including the name of the file being processed) only when an error occurs.
If we only rely on the regular output going to stderr, we also need to output
something for every file being processed, which clutters the output IMO.

But anyone serious about working on this could be expected to redirect the
output to a file!

For follow-up revisions of the patches, should I target
https://github.com/alejandro-colomar/man-pages instead of
https://git.kernel.org/pub/scm/docs/man-pages/man-pages.git?

Regards,

Stephen

[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 833 bytes --]