Обсуждение: Re: pgsql: meson: docs: Add {html,man} targets, rename install-doc-*

Hi,

This started at https://www.postgresql.org/message-id/746ba786-85bb-d1f7-b613-57bec35c642a%40dunslane.net
but seems worth discussing on -hackers.

On 2023-11-29 07:20:59 -0500, Andrew Dunstan wrote:
> On 2023-11-28 Tu 21:28, Andres Freund wrote:
> > On 2023-11-23 08:32:21 -0500, Andrew Dunstan wrote:
> > > On 2023-11-20 Mo 20:53, Andres Freund wrote:
> > > > meson: docs: Add {html,man} targets, rename install-doc-*
> > > >
> > > > We have toplevel html, man targets in the autoconf build as well. It'd be odd
> > > > to have an 'html' target but have the install target be 'install-doc-html',
> > > > thus rename the install targets to match.
> > >
> > > This commit of one of its nearby friends appears to have broken crake's docs
> > > build:
> > >
> > > ERROR: Can't invoke target `html`: ambiguous name.Add target type and/or path:
> > > - ./doc/src/sgml/html:custom
> > > - ./doc/src/sgml/html:alias
> > >
> > > See<https://buildfarm.postgresql.org/cgi-bin/show_log.pl?nm=crake&dt=2023-11-23%2012%3A52%3A04>
> > Ah, I realize now that this is from meson compile html, not 'ninja html'. That
> > explains why I couldn't reproduce this initially and why CI didn't complain.
> > I don't really understand why meson compile complains in this case.  I assume
> > you don't want to disambiguate as suggested, by building html:alias instead?
> >
>
> I've done that as a temporary fix to get crake out of the hole, but it's
> pretty ugly, and I don't want to do it in a release if at all possible.

If we want to prevent these kind of conflicts, which doesn't seem
unreasonable, I think we need an automatic check that prevents reintroducing
them. I think most people will just use ninja and not see them. Meson stores
the relevant information in meson-info/intro-targets.json, so that's just a
bit of munging of that file.

I think the background for this issue existing is that meson supports a "flat"
build directory layout (which is deprecated), so the directory name can't be
used to deconflict with meson compile, which tries to work across all "build
execution" systems.

Prototype of such a check, as well as a commit deconflicting the target names,
attached.

Greetings,

Andres Freund

Commits look fine to me, but I hate the new target names... Luckily,
I just use plain ninja, so I don't interact with that.

>  +    for name, v in targets_info_byname.items():
>  +        if len(targets_info_byname[name]) > 1:

My only comment is that you could reverse the logic and save yourself an
indentation.

- if len(targets_info_byname[name]) > 1:
+ if len(targets_info_byname[name]) <= 1:
+     continue

But whatever you want.

--
Tristan Partin
Neon (https://neon.tech)

Hi,

On 2023-12-01 15:55:29 -0600, Tristan Partin wrote:
> Commits look fine to me, but I hate the new target names...

You shouldn't ever need to use them anywhere - that's what the alias is for...

Happy to go another route if you have a suggestion.


> >  +    for name, v in targets_info_byname.items():
> >  +        if len(targets_info_byname[name]) > 1:
> 
> My only comment is that you could reverse the logic and save yourself an
> indentation.
> 
> - if len(targets_info_byname[name]) > 1:
> + if len(targets_info_byname[name]) <= 1:
> +     continue
> 
> But whatever you want.

Makes sense.