pgsql: meson: docs: Add {html,man} targets, rename install-doc-*
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.
Reviewed-by: Christoph Berg <myon@debian.org>
Reviewed-by: Peter Eisentraut <peter@eisentraut.org>
Discussion: /messages/by-id/20231103163848.26egkh5qdgw3vmil@awork3.anarazel.de
Branch
------
master
Details
-------
https://git.postgresql.org/pg/commitdiff/ddcab2a0329511e8872b62f2c77e5fa33547c277
Modified Files
--------------
doc/src/sgml/meson.build | 6 ++++--
1 file changed, 4 insertions(+), 2 deletions(-)
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>
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:aliasSee<https://buildfarm.postgresql.org/cgi-bin/show_log.pl?nm=crake&dt=2023-11-23%2012%3A52%3A04>
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:aliasSee<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?
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:aliasSee<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.
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:aliasSee<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
Attachments:
v1-0001-meson-Rename-target-names-that-conflict-with-glob.patchtext/x-diff; charset=us-asciiDownload
From 7b44707b9caa4528f1a6e8dd218644fa114007b3 Mon Sep 17 00:00:00 2001
From: Andres Freund <andres@anarazel.de>
Date: Wed, 29 Nov 2023 10:33:47 -0800
Subject: [PATCH v1 1/2] meson: Rename target names that conflict with global
target names
Author:
Reviewed-by:
Discussion: https://postgr.es/m/703110e4-b495-e409-26a5-28e9fca8f3a5@dunslane.net
Backpatch:
---
src/common/unicode/meson.build | 2 +-
doc/src/sgml/meson.build | 6 +++---
2 files changed, 4 insertions(+), 4 deletions(-)
diff --git a/src/common/unicode/meson.build b/src/common/unicode/meson.build
index 6af46122c4e..c548710c293 100644
--- a/src/common/unicode/meson.build
+++ b/src/common/unicode/meson.build
@@ -139,7 +139,7 @@ endif
# Use a custom target, as run targets serialize the output, making this harder
# to debug, and don't deal well with targets with multiple outputs.
-update_unicode = custom_target('update-unicode',
+update_unicode = custom_target('tgt-update-unicode',
depends: update_unicode_dep,
output: ['dont-exist'],
input: update_unicode_targets,
diff --git a/doc/src/sgml/meson.build b/doc/src/sgml/meson.build
index e1a85dc607b..c604bdbd644 100644
--- a/doc/src/sgml/meson.build
+++ b/doc/src/sgml/meson.build
@@ -135,7 +135,7 @@ endif
# Full documentation as html, text
#
if docs_dep.found()
- html = custom_target('html',
+ html = custom_target('tgt-build-html',
input: ['stylesheet.xsl', postgres_full_xml],
output: 'html',
depfile: 'html.d',
@@ -144,7 +144,7 @@ if docs_dep.found()
)
alldocs += html
- install_doc_html = custom_target('install-html',
+ install_doc_html = custom_target('tgt-install-html',
output: 'install-html',
command: [
python, install_files, '--prefix', dir_prefix,
@@ -224,7 +224,7 @@ endif
#
if docs_dep.found()
# FIXME: implement / consider sqlmansectnum logic
- man = custom_target('man',
+ man = custom_target('tgt-build-man',
input: ['stylesheet-man.xsl', postgres_full_xml],
output: ['man1', 'man3', 'man7'],
depfile: 'man.d',
--
2.38.0
v1-0002-meson-Add-test-checking-if-there-are-conflicting-.patchtext/x-diff; charset=us-asciiDownload
From a15cc895abfc9b426af75051d069fd625391b618 Mon Sep 17 00:00:00 2001
From: Andres Freund <andres@anarazel.de>
Date: Wed, 29 Nov 2023 10:30:21 -0800
Subject: [PATCH v1 2/2] meson: Add test checking if there are conflicting
target names
Author:
Reviewed-by:
Discussion: https://postgr.es/m/703110e4-b495-e409-26a5-28e9fca8f3a5@dunslane.net
Backpatch:
---
meson.build | 8 ++++++
src/tools/check_meson | 67 +++++++++++++++++++++++++++++++++++++++++++
2 files changed, 75 insertions(+)
create mode 100755 src/tools/check_meson
diff --git a/meson.build b/meson.build
index 0095fb183af..e0895f41f78 100644
--- a/meson.build
+++ b/meson.build
@@ -3320,6 +3320,14 @@ if meson.version().version_compare('>=0.57')
endif
+# Tests verifing that source code follows certain rules
+test('meson-check',
+ files('src/tools/check_meson'),
+ args: ['--srcdir', meson.source_root(), '--builddir', meson.build_root()],
+ suite: 'source',
+ priority: 10)
+
+
###############################################################
# Pseudo targets
diff --git a/src/tools/check_meson b/src/tools/check_meson
new file mode 100755
index 00000000000..060ccd269ff
--- /dev/null
+++ b/src/tools/check_meson
@@ -0,0 +1,67 @@
+#!/usr/bin/env python3
+
+# Script to perform some consistency checks on the meson build.
+
+from collections import defaultdict
+import argparse
+import json
+import os
+import sys
+
+
+def check_target_names(args):
+ """
+ Check that the target names for run_target() and alias_target() do
+ not conflict with other target types like
+ custom_target(). alias_target()/run_target() targets are intended to
+ be invoked at the top-level, but "meson compile $target" complains
+ about conflicts if another name of the same target exists.
+ """
+ targets_info_path = os.path.join(
+ args.builddir, 'meson-info/intro-targets.json')
+ targets_info = json.load(open(targets_info_path))
+
+ targets_info_byname = defaultdict(list)
+
+ for r in targets_info:
+ targets_info_byname[r['name']].append(r)
+
+ have_conflicts = False
+
+ for name, v in targets_info_byname.items():
+ if len(targets_info_byname[name]) > 1:
+ dirs = set()
+ types = set()
+ have_target_conflict = False
+ for t in v:
+ dirs.add(t['defined_in'])
+ types.add(t['type'])
+ if len(dirs) < len(v) and ('alias' in types or 'run' in types):
+ have_target_conflict = True
+
+ if have_target_conflict:
+ have_conflicts = True
+ print(f'Global target "{name}" has conflicting target names:',
+ file=sys.stderr)
+ for t in v:
+ fname = os.path.relpath(t['defined_in'], args.srcdir)
+ print(f'\t"{t["name"]}:{t["type"]}", defined in "{fname}"',
+ file=sys.stderr)
+ print(file=sys.stderr)
+
+ if have_conflicts:
+ print("Please rename conflicting targets",
+ file=sys.stderr)
+ return False
+ return True
+
+
+if __name__ == '__main__':
+ parser = argparse.ArgumentParser()
+ parser.add_argument('--builddir', type=str, required=True)
+ parser.add_argument('--srcdir', type=str, required=True)
+ args = parser.parse_args()
+
+ all_ok = True
+ all_ok &= check_target_names(args)
+ sys.exit(not all_ok)
--
2.38.0
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:aliasSee<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.
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:aliasSee<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.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:aliasSee<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.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:
+ continueBut 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: + continueBut whatever you want.
Makes sense.