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

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&amp;dt=2023-11-23%2012%3A52%3A04&gt;

cheers

andrew

--
Andrew Dunstan
EDB:https://www.enterprisedb.com

On 2023-11-23 Th 08:32, 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&amp;dt=2023-11-23%2012%3A52%3A04&gt;

This is still broken.

cheers

andrew

--
Andrew Dunstan
EDB:https://www.enterprisedb.com

Hi,

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&amp;dt=2023-11-23%2012%3A52%3A04&gt;

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?

Greetings,

Andres Freund

On 2023-11-28 Tu 21:28, Andres Freund wrote:

Hi,

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&amp;dt=2023-11-23%2012%3A52%3A04&gt;

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.

cheers

andrew

--
Andrew Dunstan
EDB: https://www.enterprisedb.com

Andrew Dunstan <andrew@dunslane.net> writes:

On 2023-11-28 Tu 21:28, Andres Freund wrote:

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.

Our documentation says specifically that "ninja html" will build the
HTML format. I would expect that to work by analogy with the "make"
target; having to spell it differently seems like clearly a bug.

regards, tom lane

On 2023-11-29 We 08:49, Tom Lane wrote:

Andrew Dunstan <andrew@dunslane.net> writes:

On 2023-11-28 Tu 21:28, Andres Freund wrote:

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.

Our documentation says specifically that "ninja html" will build the
HTML format. I would expect that to work by analogy with the "make"
target; having to spell it differently seems like clearly a bug.

"ninja html" does in fact work. What's not working is "meson compile
html". And it looks like the reason I used that in the buildfarm code is
that ninja doesn't know about other targets like "postgres-US.pdf". Up
to now "meson compile postgres-US.pdf html" has worked.

FWIW, the buildfarm code doesn't use ninja explicitly anywhere else.

cheers

andrew

--
Andrew Dunstan
EDB: https://www.enterprisedb.com

Hi,

On 2023-11-29 10:05:26 -0500, Andrew Dunstan wrote:

On 2023-11-29 We 08:49, Tom Lane wrote:

Andrew Dunstan <andrew@dunslane.net> writes:

On 2023-11-28 Tu 21:28, Andres Freund wrote:

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.

Our documentation says specifically that "ninja html" will build the
HTML format. I would expect that to work by analogy with the "make"
target; having to spell it differently seems like clearly a bug.

"ninja html" does in fact work. What's not working is "meson compile html".
And it looks like the reason I used that in the buildfarm code is that ninja
doesn't know about other targets like "postgres-US.pdf".

It does:

ninja help|grep pdf
doc/src/sgml/postgres-A4.pdf Build documentation in PDF format, with A4 pages
doc/src/sgml/postgres-US.pdf Build documentation in PDF format, with US letter pages

"ninja doc/src/sgml/postgres-US.pdf" works and has worked since day one.

FWIW, you can continue to use meson compile, you just need to disambiguate the
target name:
meson compile html:alias

Which isn't particularly pretty, but does work.

Greetings,

Andres Freund

Hi,

This started at /messages/by-id/746ba786-85bb-d1f7-b613-57bec35c642a@dunslane.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&amp;dt=2023-11-23%2012%3A52%3A04&gt;

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

On 2023-11-29 We 07:20, Andrew Dunstan wrote:

On 2023-11-28 Tu 21:28, Andres Freund wrote:

Hi,

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&amp;dt=2023-11-23%2012%3A52%3A04&gt;

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.

and doing this has broken the docs build for release 16.

cheers

andrew

--
Andrew Dunstan
EDB: https://www.enterprisedb.com

Hi,

On 2023-12-01 09:04:19 -0500, Andrew Dunstan wrote:

On 2023-11-29 We 07:20, 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&amp;dt=2023-11-23%2012%3A52%3A04&gt;

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.

and doing this has broken the docs build for release 16.

If I can get somebody to comment on
/messages/by-id/20231129183619.3hrnwaexbrpygbxg@awork3.anarazel.de
we can remove the need for the :$buildtype suffix.

Greetings,

Andres Freund

On 2023-12-01 Fr 09:04, Andrew Dunstan wrote:

On 2023-11-29 We 07:20, Andrew Dunstan wrote:

On 2023-11-28 Tu 21:28, Andres Freund wrote:

Hi,

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&amp;dt=2023-11-23%2012%3A52%3A04&gt;

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.

and doing this has broken the docs build for release 16.

OK, so this code is what I have now, and seems to work on both HEAD and
REL_16_STABLE:

        my $extra_targets = $PGBuild::conf{extra_doc_targets} || "";
        my @targs = split(/\s+/, $extra_targets);
        s!^!doc/src/sgml/! foreach @targs;
        $extra_targets=join(' ', @targs) ;
        @makeout = run_log("cd $pgsql && ninja doc/src/sgml/html $extra_targets");

cheers

andrew

--
Andrew Dunstan
EDB:https://www.enterprisedb.com

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.