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
>From 7b44707b9caa4528f1a6e8dd218644fa114007b3 Mon Sep 17 00:00:00 2001
From: Andres Freund <[email protected]>
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/[email protected]
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
>From a15cc895abfc9b426af75051d069fd625391b618 Mon Sep 17 00:00:00 2001
From: Andres Freund <[email protected]>
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/[email protected]
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
