meson missing test dependencies
I have noticed that under meson many tests don't have dependencies on
the build artifacts that they are testing. As an example among many, if
you make a source code change in contrib/cube/cube.c (see patch 0001 for
a demo) and then run
make -C contrib/cube check
the test run will reflect the changed code, because the "check" targets
typically depend on the "all" targets. But if you do this under meson with
meson test -C build --suite setup --suite cube
the code will not be rebuilt first, and the test run will not reflect
the changed code.
This seems straightforward to fix, see patch 0002. The meson test setup
has support for this, but it seems not widely used.
Patch 0003 is another example, this time for a TAP-style test.
Is there any reason this was not done yet?
Attachments:
v0-0001-XXX-source-code-modification-for-test-demonstrati.patchtext/plain; charset=UTF-8; name=v0-0001-XXX-source-code-modification-for-test-demonstrati.patchDownload
From 079a7f902177f5e146c004fb7789c9972c86c5ae Mon Sep 17 00:00:00 2001
From: Peter Eisentraut <peter@eisentraut.org>
Date: Mon, 2 Dec 2024 10:48:37 +0100
Subject: [PATCH v0 1/3] XXX source code modification for test demonstration
---
contrib/cube/cube.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/contrib/cube/cube.c b/contrib/cube/cube.c
index 1fc447511a1..e3ad25a2a39 100644
--- a/contrib/cube/cube.c
+++ b/contrib/cube/cube.c
@@ -278,7 +278,7 @@ cube_subset(PG_FUNCTION_ARGS)
if ((dx[i] <= 0) || (dx[i] > DIM(c)))
ereport(ERROR,
(errcode(ERRCODE_ARRAY_ELEMENT_ERROR),
- errmsg("Index out of bounds")));
+ errmsg("Index out of bounds XXX")));
result->x[i] = c->x[dx[i] - 1];
if (!IS_POINT(c))
result->x[i + dim] = c->x[dx[i] + DIM(c) - 1];
--
2.47.1
v0-0002-Add-test-dependency.patchtext/plain; charset=UTF-8; name=v0-0002-Add-test-dependency.patchDownload
From 00b367c86dd14840d8412259f098f09737e73205 Mon Sep 17 00:00:00 2001
From: Peter Eisentraut <peter@eisentraut.org>
Date: Mon, 2 Dec 2024 10:48:37 +0100
Subject: [PATCH v0 2/3] Add test dependency
---
contrib/cube/meson.build | 1 +
1 file changed, 1 insertion(+)
diff --git a/contrib/cube/meson.build b/contrib/cube/meson.build
index 21b6f9c43ad..64ba300495d 100644
--- a/contrib/cube/meson.build
+++ b/contrib/cube/meson.build
@@ -53,6 +53,7 @@ tests += {
'sd': meson.current_source_dir(),
'bd': meson.current_build_dir(),
'regress': {
+ 'deps': [cube],
'sql': [
'cube',
'cube_sci',
--
2.47.1
v0-0003-Add-test-dependency-for-TAP-module.patchtext/plain; charset=UTF-8; name=v0-0003-Add-test-dependency-for-TAP-module.patchDownload
From 176dc1eb383305025a3de6a895825de36f197678 Mon Sep 17 00:00:00 2001
From: Peter Eisentraut <peter@eisentraut.org>
Date: Mon, 2 Dec 2024 10:48:37 +0100
Subject: [PATCH v0 3/3] Add test dependency for TAP module
---
src/test/modules/test_json_parser/meson.build | 3 +++
1 file changed, 3 insertions(+)
diff --git a/src/test/modules/test_json_parser/meson.build b/src/test/modules/test_json_parser/meson.build
index 059a8b71bde..f5a8ea44eed 100644
--- a/src/test/modules/test_json_parser/meson.build
+++ b/src/test/modules/test_json_parser/meson.build
@@ -61,5 +61,8 @@ tests += {
't/003_test_semantic.pl',
't/004_test_parser_perf.pl'
],
+ 'test_kwargs': {
+ 'depends': [test_json_parser_incremental, test_json_parser_perf],
+ },
},
}
--
2.47.1
Hi,
On Mon, 2 Dec 2024 at 13:11, Peter Eisentraut <peter@eisentraut.org> wrote:
I have noticed that under meson many tests don't have dependencies on
the build artifacts that they are testing. As an example among many, if
you make a source code change in contrib/cube/cube.c (see patch 0001 for
a demo) and then runmake -C contrib/cube check
the test run will reflect the changed code, because the "check" targets
typically depend on the "all" targets. But if you do this under meson withmeson test -C build --suite setup --suite cube
the code will not be rebuilt first, and the test run will not reflect
the changed code.This seems straightforward to fix, see patch 0002. The meson test setup
has support for this, but it seems not widely used.Patch 0003 is another example, this time for a TAP-style test.
Is there any reason this was not done yet?
This looks useful. I am not sure why this was not done before.
I applied your patches and the cube example worked. But when I edited
'test_json_parser_perf.c' and 'test_json_parser_incremental.c', meson
did not rebuild. I used the 'meson test -C build --suite setup --suite
test_json_parser' command to test test_json_parser. Did I do something
wrong?
--
Regards,
Nazir Bilal Yavuz
Microsoft
On 02.12.24 11:50, Nazir Bilal Yavuz wrote:
I applied your patches and the cube example worked. But when I edited
'test_json_parser_perf.c' and 'test_json_parser_incremental.c', meson
did not rebuild. I used the 'meson test -C build --suite setup --suite
test_json_parser' command to test test_json_parser. Did I do something
wrong?
Seems to work for me. I edited test_json_parser_incremental.c and then ran
$ meson test -C build --suite setup --suite test_json_parser -v
ninja: Entering directory `/Users/peter/devel/postgresql/postgresql/build'
[2/2] Linking target
src/test/modules/test_json_parser/test_json_parser_incremental
1/7 postgresql:setup / tmp_install
RUNNING
...
Without my patch, you don't get the "Linking target ..." output.
On Mon, Dec 02, 2024 at 01:50:57PM +0300, Nazir Bilal Yavuz wrote:
On Mon, 2 Dec 2024 at 13:11, Peter Eisentraut <peter@eisentraut.org> wrote:
Is there any reason this was not done yet?
This looks useful. I am not sure why this was not done before.
Do you think that it would be possible to automate the addition of the
dependency link between the module and the tests in some way? The
most common case where the only dependency needed by the test is the
module itself would cover a lot of ground if this could be enforced in
some fashion.
--
Michael
Hi,
On Mon, 2 Dec 2024 at 22:27, Peter Eisentraut <peter@eisentraut.org> wrote:
On 02.12.24 11:50, Nazir Bilal Yavuz wrote:
I applied your patches and the cube example worked. But when I edited
'test_json_parser_perf.c' and 'test_json_parser_incremental.c', meson
did not rebuild. I used the 'meson test -C build --suite setup --suite
test_json_parser' command to test test_json_parser. Did I do something
wrong?Seems to work for me. I edited test_json_parser_incremental.c and then ran
$ meson test -C build --suite setup --suite test_json_parser -v
ninja: Entering directory `/Users/peter/devel/postgresql/postgresql/build'
[2/2] Linking target
src/test/modules/test_json_parser/test_json_parser_incremental
1/7 postgresql:setup / tmp_install
RUNNING
...Without my patch, you don't get the "Linking target ..." output.
It is working fine now, I must have done something wrong before. Sorry
for the noise.
--
Regards,
Nazir Bilal Yavuz
Microsoft
Hi,
On Tue, 3 Dec 2024 at 04:05, Michael Paquier <michael@paquier.xyz> wrote:
On Mon, Dec 02, 2024 at 01:50:57PM +0300, Nazir Bilal Yavuz wrote:
On Mon, 2 Dec 2024 at 13:11, Peter Eisentraut <peter@eisentraut.org> wrote:
Is there any reason this was not done yet?
This looks useful. I am not sure why this was not done before.
Do you think that it would be possible to automate the addition of the
dependency link between the module and the tests in some way? The
most common case where the only dependency needed by the test is the
module itself would cover a lot of ground if this could be enforced in
some fashion.
I tried something like:
diff --git a/meson.build b/meson.build
index 451c3f6d851..ddf6045d472 100644
--- a/meson.build
+++ b/meson.build
@@ -3366,6 +3366,13 @@ foreach test_dir : testst = test_dir[kind]
+ t_dep = [get_variable(test_dir['name'], '')]
+ message('Adding test = @0@ and dep = @1@'.format(test_dir['name'], t_dep))
+ if t_dep != ['']
+ t += {'deps': t_dep}
+ endif
+The code above creates a dependency between the module (*whose name is
same with the test*) and the test. This errors out for the 'libpq,
ssl, ldap and icu' because the type of these variables is dependency;
but test.depends can be one of the '[BuildTarget | CustomTarget |
CustomTargetIndex]' types, it can not be a dependency type.
And this can not create a link for the 'scripts, regress, isolation,
authentication, postmaster, recovery, subscription, brin, commit_ts,
gin, test_extensions, test_json_parser, test_misc, test_pg_dump,
typcache, unsafe_tests, worker_spi, kerberos and ecpg' tests. I think
only 'regress, isolation, test_json_parser, worker_spi and ecpg' are
wrong in this list as their modules names are not the same with their
tests.
--
Regards,
Nazir Bilal Yavuz
Microsoft
Hi,
On 2024-12-02 11:10:56 +0100, Peter Eisentraut wrote:
I have noticed that under meson many tests don't have dependencies on the
build artifacts that they are testing. As an example among many, if you
make a source code change in contrib/cube/cube.c (see patch 0001 for a demo)
and then runmake -C contrib/cube check
the test run will reflect the changed code, because the "check" targets
typically depend on the "all" targets. But if you do this under meson withmeson test -C build --suite setup --suite cube
the code will not be rebuilt first, and the test run will not reflect the
changed code.
That's unfortunately a very partial fix - because we insist on tests being run
against a temporary install, we don't just need to rebuild the code, we also
need to re-install it. Without that you don't, e.g., see server changes.
Currently the install is done as part of the setup/tmp_install test.
It'd be *much* nicer to create the temporary installation as a build step, but
unfortunately meson currently builds all test dependencies when you build the
default target. Which unfortunately would mean that we'd reinstall the temp
installation whenever 'ninja' is issued. Hence the weird setup with the test
doing the install. There's recent progress towards improving this in meson,
luckily.
However, it looks like the tmp_install test *does* miss dependencies too and I
see no reason to not fix that.
Medium term I think we should work on being able to run our tests from the
source tree. That'd substantially improve the time to run individual tests, I
think.
Greetings,
Andres Freund
diff --git i/meson.build w/meson.build
index ff3848b1d85..55b751a0c6b 100644
--- i/meson.build
+++ w/meson.build
@@ -3263,6 +3263,7 @@ test('tmp_install',
priority: setup_tests_priority,
timeout: 300,
is_parallel: false,
+ depends: all_built,
suite: ['setup'])test('install_test_files',
On 03.12.24 17:01, Andres Freund wrote:
Hi,
On 2024-12-02 11:10:56 +0100, Peter Eisentraut wrote:
I have noticed that under meson many tests don't have dependencies on the
build artifacts that they are testing. As an example among many, if you
make a source code change in contrib/cube/cube.c (see patch 0001 for a demo)
and then runmake -C contrib/cube check
the test run will reflect the changed code, because the "check" targets
typically depend on the "all" targets. But if you do this under meson withmeson test -C build --suite setup --suite cube
the code will not be rebuilt first, and the test run will not reflect the
changed code.That's unfortunately a very partial fix - because we insist on tests being run
against a temporary install, we don't just need to rebuild the code, we also
need to re-install it. Without that you don't, e.g., see server changes.
Right, I was willing to accept that as a compromise, but see below.
However, it looks like the tmp_install test *does* miss dependencies too and I
see no reason to not fix that.
diff --git i/meson.build w/meson.build index ff3848b1d85..55b751a0c6b 100644 --- i/meson.build +++ w/meson.build @@ -3263,6 +3263,7 @@ test('tmp_install', priority: setup_tests_priority, timeout: 300, is_parallel: false, + depends: all_built, suite: ['setup'])test('install_test_files',
Yes, that addresses my cube example.
But it doesn't address the test_json_parser example, because in that
case the test executables are not installed. So in that case we still
need to manually add the dependencies. But those are only a few cases
(maybe just test_json_parser and libpq_pipeline).
Hi,
On 2024-12-05 20:08:24 +0100, Peter Eisentraut wrote:
On 03.12.24 17:01, Andres Freund wrote:
On 2024-12-02 11:10:56 +0100, Peter Eisentraut wrote:
That's unfortunately a very partial fix - because we insist on tests being run
against a temporary install, we don't just need to rebuild the code, we also
need to re-install it. Without that you don't, e.g., see server changes.Right, I was willing to accept that as a compromise, but see below.
However, it looks like the tmp_install test *does* miss dependencies too and I
see no reason to not fix that.diff --git i/meson.build w/meson.build index ff3848b1d85..55b751a0c6b 100644 --- i/meson.build +++ w/meson.build @@ -3263,6 +3263,7 @@ test('tmp_install', priority: setup_tests_priority, timeout: 300, is_parallel: false, + depends: all_built, suite: ['setup'])test('install_test_files',
Yes, that addresses my cube example.
Let's do that. I'm inclined to only do so on master, but I can also see
backpatching it. Arguments?
But it doesn't address the test_json_parser example, because in that case
the test executables are not installed. So in that case we still need to
manually add the dependencies. But those are only a few cases (maybe just
test_json_parser and libpq_pipeline).
Agreed!
Greetings,
Andres Freund
On 05.12.24 21:10, Andres Freund wrote:
Hi,
On 2024-12-05 20:08:24 +0100, Peter Eisentraut wrote:
On 03.12.24 17:01, Andres Freund wrote:
On 2024-12-02 11:10:56 +0100, Peter Eisentraut wrote:
That's unfortunately a very partial fix - because we insist on tests being run
against a temporary install, we don't just need to rebuild the code, we also
need to re-install it. Without that you don't, e.g., see server changes.Right, I was willing to accept that as a compromise, but see below.
However, it looks like the tmp_install test *does* miss dependencies too and I
see no reason to not fix that.diff --git i/meson.build w/meson.build index ff3848b1d85..55b751a0c6b 100644 --- i/meson.build +++ w/meson.build @@ -3263,6 +3263,7 @@ test('tmp_install', priority: setup_tests_priority, timeout: 300, is_parallel: false, + depends: all_built, suite: ['setup'])test('install_test_files',
Yes, that addresses my cube example.
Let's do that. I'm inclined to only do so on master, but I can also see
backpatching it. Arguments?
master is enough for me.
Hi,
On 2024-12-06 18:54:25 +0100, Peter Eisentraut wrote:
On 05.12.24 21:10, Andres Freund wrote:
On 2024-12-05 20:08:24 +0100, Peter Eisentraut wrote:
On 03.12.24 17:01, Andres Freund wrote:
On 2024-12-02 11:10:56 +0100, Peter Eisentraut wrote:
That's unfortunately a very partial fix - because we insist on tests being run
against a temporary install, we don't just need to rebuild the code, we also
need to re-install it. Without that you don't, e.g., see server changes.Right, I was willing to accept that as a compromise, but see below.
However, it looks like the tmp_install test *does* miss dependencies too and I
see no reason to not fix that.diff --git i/meson.build w/meson.build index ff3848b1d85..55b751a0c6b 100644 --- i/meson.build +++ w/meson.build @@ -3263,6 +3263,7 @@ test('tmp_install', priority: setup_tests_priority, timeout: 300, is_parallel: false, + depends: all_built, suite: ['setup'])test('install_test_files',
Yes, that addresses my cube example.
Let's do that. I'm inclined to only do so on master, but I can also see
backpatching it. Arguments?master is enough for me.
This doesn't actually make that much difference. But only really kinda by
accident, so I'm still planning to apply a patch improving this.
If a test target does not have ay dependency 'meson test' treats it has having
a dependency on the project default test. Which in turn currently includes
everything that will be installed, i.e. everything that tmp_install might
need. But that's not something I think we should rely on going forward.
Attached is a series that:
- Reduces the dependencies of the 'install-quiet' target, to not build
e.g. ecpg tests
- Fixes the dependencies of tmp_install test more precise
- Fixes dependencies for ecpg's tests
- Fixes dependencies for test_json_parser's (slightly evolved version of
yours)
- Fixes dependencies for libpq_pipeline
- Fixes dependencies for libpq's tests
The last case was a bit less fun to fix.
To have correct dependencies, the libpq tests need a dependency on
libpq_uri_regress and libpq_testclient. But until now that was hard to do,
because src/interfaces/libpq/test is entered from the top-level:
# test/ is entered via top-level meson.build, that way it can use the default
# args for executables (which depend on libpq).
At the top-level we have:
subdir('src/interfaces/libpq')
# fe_utils depends on libpq
subdir('src/fe_utils')
# for frontend binaries
frontend_code = declare_dependency(
include_directories: [postgres_inc],
link_with: [fe_utils, common_static, pgport_static],
sources: generated_headers,
dependencies: [os_deps, libintl],
)
and the test binaries currently use frontend_code.
The least bad fix that I could see was to introduce, at the top-level,
frontend_no_fe_utils_code, which can be defined before we get to libpq/. That
in turn allows us to recurse into libpq/test from within libpq/meson.build,
which allows us to define the tests with proper dependencies.
With all of these applied
ninja clean && ninja meson-test-prereq && m test --no-rebuild
passes, which doesn't guarantee that *all* dependencies are correctly
declared, but certainly is better than where we were before.
Greetings,
Andres Freund
Attachments:
v1-0001-meson-Narrow-dependencies-for-install-quiet-targe.patchtext/x-diff; charset=us-asciiDownload
From dff957eb1b5f0c45e552affede99b3585bbcca00 Mon Sep 17 00:00:00 2001
From: Andres Freund <andres@anarazel.de>
Date: Sat, 7 Dec 2024 14:33:29 -0500
Subject: [PATCH v1 1/6] meson: Narrow dependencies for 'install-quiet' target
Previously test dependencies, which are not actually installed, were
unnecessarily built.
Author:
Reviewed-by:
Discussion: https://postgr.es/m/
Backpatch:
---
meson.build | 12 +++++++++---
1 file changed, 9 insertions(+), 3 deletions(-)
diff --git a/meson.build b/meson.build
index e5ce437a5c7..87ba82b27b0 100644
--- a/meson.build
+++ b/meson.build
@@ -3185,24 +3185,30 @@ if libintl.found() and meson.version().version_compare('>=0.60')
endif
-all_built = [
+# all targets that 'meson install' needs
+installed_targets = [
backend_targets,
bin_targets,
libpq_st,
pl_targets,
contrib_targets,
nls_mo_targets,
- testprep_targets,
ecpg_targets,
]
+# all targets that require building code
+all_built = [
+ installed_targets,
+ testprep_targets,
+]
+
# Meson's default install target is quite verbose. Provide one that is quiet.
install_quiet = custom_target('install-quiet',
output: 'install-quiet',
build_always_stale: true,
build_by_default: false,
command: [meson_bin, meson_args, 'install', '--quiet', '--no-rebuild'],
- depends: all_built,
+ depends: installed_targets,
)
# Target to install files used for tests, which aren't installed by default
--
2.45.2.746.g06e570c0df.dirty
v1-0002-meson-Improve-dependencies-for-tmp_install-test-t.patchtext/x-diff; charset=us-asciiDownload
From fdf058d1b9255ee76dfa7129d4e7d9c4e9a726b6 Mon Sep 17 00:00:00 2001
From: Andres Freund <andres@anarazel.de>
Date: Sat, 7 Dec 2024 14:29:03 -0500
Subject: [PATCH v1 2/6] meson: Improve dependencies for tmp_install test
target
The missing dependency was, e.g., visible when doing
ninja clean && ninja meson-test-prereq && meson test --no-rebuild --suite setup --suite cube
because meson (and thus its internal meson-test-prereq target) did not know
about a lot of the required targets.
Previously tmp_install did not actually depend on the relevant files being
built. That was mostly not visible, because "meson test" currently uses the
'default' targets as a test's dependency if no dependency is specified.
However, there are plans to narrow that on the meson side, to make it quicker
to run tests.
Discussion: https://postgr.es/m/bdba588f-69a9-4f3e-9b95-62d07210a32e@eisentraut.org
---
meson.build | 1 +
1 file changed, 1 insertion(+)
diff --git a/meson.build b/meson.build
index 87ba82b27b0..e74b1b9b161 100644
--- a/meson.build
+++ b/meson.build
@@ -3263,6 +3263,7 @@ test('tmp_install',
priority: setup_tests_priority,
timeout: 300,
is_parallel: false,
+ depends: installed_targets,
suite: ['setup'])
test('install_test_files',
--
2.45.2.746.g06e570c0df.dirty
v1-0003-meson-Add-pg_regress_ecpg-to-ecpg-test-dependenci.patchtext/x-diff; charset=us-asciiDownload
From d6e6e0a23638d7860545c9ff2bf0ccd752885e83 Mon Sep 17 00:00:00 2001
From: Andres Freund <andres@anarazel.de>
Date: Sat, 7 Dec 2024 14:22:18 -0500
Subject: [PATCH v1 3/6] meson: Add pg_regress_ecpg to ecpg test dependencies
This is required to ensure correct test dependencies, previously
pg_regress_ecpg would not necessarily be built.
The missing dependency was, e.g., visible when doing
ninja clean && ninja meson-test-prereq && meson test --no-rebuild --suite setup --suite ecpg
Author:
Reviewed-by:
Discussion: https://postgr.es/m/
Backpatch:
---
src/interfaces/ecpg/test/meson.build | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/src/interfaces/ecpg/test/meson.build b/src/interfaces/ecpg/test/meson.build
index 8fd284071f2..c10c653cd82 100644
--- a/src/interfaces/ecpg/test/meson.build
+++ b/src/interfaces/ecpg/test/meson.build
@@ -5,6 +5,8 @@ if meson.is_cross_build()
subdir_done()
endif
+ecpg_test_dependencies = []
+
pg_regress_ecpg_sources = pg_regress_c + files(
'pg_regress_ecpg.c',
)
@@ -23,7 +25,7 @@ pg_regress_ecpg = executable('pg_regress_ecpg',
'install': false
},
)
-testprep_targets += pg_regress_ecpg
+ecpg_test_dependencies += pg_regress_ecpg
# create .c files and executables from .pgc files
ecpg_test_exec_kw = {
@@ -51,8 +53,6 @@ ecpg_preproc_test_command_end = [
'-o', '@OUTPUT@', '@INPUT@'
]
-ecpg_test_dependencies = []
-
subdir('compat_informix')
subdir('compat_oracle')
subdir('connect')
--
2.45.2.746.g06e570c0df.dirty
v1-0004-meson-Add-test-dependencies-for-test_json_parser.patchtext/x-diff; charset=us-asciiDownload
From 88c0e3f9f6ea1f1aad4c5de4436ebde40a9c4c7e Mon Sep 17 00:00:00 2001
From: Andres Freund <andres@anarazel.de>
Date: Sat, 7 Dec 2024 15:00:35 -0500
Subject: [PATCH v1 4/6] meson: Add test dependencies for test_json_parser
This is required to ensure correct test dependencies, previously
the test binaries would not necessarily be built.
The missing dependency was, e.g., visible when doing
ninja clean && ninja meson-test-prereq && m test --no-rebuild --suite setup --suite test_json_parser
Author: Peter Eisentraut <peter@eisentraut.org>
Author: Andres Freund <andres@anarazel.de>
Discussion: https://postgr.es/m/bdba588f-69a9-4f3e-9b95-62d07210a32e@eisentraut.org
---
src/test/modules/test_json_parser/meson.build | 5 +++++
1 file changed, 5 insertions(+)
diff --git a/src/test/modules/test_json_parser/meson.build b/src/test/modules/test_json_parser/meson.build
index 059a8b71bde..1bc10f11bbf 100644
--- a/src/test/modules/test_json_parser/meson.build
+++ b/src/test/modules/test_json_parser/meson.build
@@ -61,5 +61,10 @@ tests += {
't/003_test_semantic.pl',
't/004_test_parser_perf.pl'
],
+ 'deps': [
+ test_json_parser_incremental,
+ test_json_parser_incremental_shlib,
+ test_json_parser_perf,
+ ],
},
}
--
2.45.2.746.g06e570c0df.dirty
v1-0005-meson-Add-missing-dependencies-to-libpq_pipeline-.patchtext/x-diff; charset=us-asciiDownload
From 23e9200b266093cd65f895b647af76b867d71d86 Mon Sep 17 00:00:00 2001
From: Andres Freund <andres@anarazel.de>
Date: Sat, 7 Dec 2024 14:41:20 -0500
Subject: [PATCH v1 5/6] meson: Add missing dependencies to libpq_pipeline test
The missing dependency was, e.g., visible when doing
ninja clean && ninja meson-test-prereq && meson test --no-rebuild --suite setup --suite libpq_pipeline
---
src/test/modules/libpq_pipeline/meson.build | 1 +
1 file changed, 1 insertion(+)
diff --git a/src/test/modules/libpq_pipeline/meson.build b/src/test/modules/libpq_pipeline/meson.build
index 727963ee686..43beda411ce 100644
--- a/src/test/modules/libpq_pipeline/meson.build
+++ b/src/test/modules/libpq_pipeline/meson.build
@@ -27,5 +27,6 @@ tests += {
'tests': [
't/001_libpq_pipeline.pl',
],
+ 'deps': [libpq_pipeline],
},
}
--
2.45.2.746.g06e570c0df.dirty
v1-0006-meson-Add-missing-dependencies-for-libpq-tests.patchtext/x-diff; charset=us-asciiDownload
From 14aefeae08975198abc73a073b15559e73ac3774 Mon Sep 17 00:00:00 2001
From: Andres Freund <andres@anarazel.de>
Date: Sat, 7 Dec 2024 15:30:21 -0500
Subject: [PATCH v1 6/6] meson: Add missing dependencies for libpq tests
The missing dependency was, e.g., visible when doing
ninja clean && ninja meson-test-prereq && meson test --no-rebuild --suite setup --suite libpq
This is a bit more complicated than other related fixes, because until now
libpq's tests depended on 'frontend_code', which includes a dependency on
fe_utils, which in turns on libpq. That in turn required
src/interfaces/libpq/test to be entered from the top-level, not from
libpq/meson.build. Because of that the test definitions in libpq/meson.build
could not declare a dependency on the binaries defined in
libpq/test/meson.build.
To fix this, this commit creates frontend_no_fe_utils_code, which allows us to
recurse into libpq/test from withing libpq/meson.build.
---
meson.build | 11 ++++++++++-
src/interfaces/libpq/meson.build | 5 ++---
src/interfaces/libpq/test/meson.build | 12 ++++++++----
3 files changed, 20 insertions(+), 8 deletions(-)
diff --git a/meson.build b/meson.build
index e74b1b9b161..d25070979a1 100644
--- a/meson.build
+++ b/meson.build
@@ -3024,6 +3024,16 @@ frontend_shlib_code = declare_dependency(
dependencies: [shlib_code, os_deps, libintl],
)
+# for frontend code that doesn't use fe_utils - this mainly exists for libpq's
+# tests, which are defined before fe_utils is defined, as fe_utils depends on
+# libpq.
+frontend_no_fe_utils_code = declare_dependency(
+ include_directories: [postgres_inc],
+ link_with: [common_static, pgport_static],
+ sources: generated_headers,
+ dependencies: [os_deps, libintl],
+)
+
# Dependencies both for static and shared libpq
libpq_deps += [
thread_dep,
@@ -3091,7 +3101,6 @@ subdir('src')
subdir('contrib')
subdir('src/test')
-subdir('src/interfaces/libpq/test')
subdir('src/interfaces/ecpg/test')
subdir('doc/src/sgml')
diff --git a/src/interfaces/libpq/meson.build b/src/interfaces/libpq/meson.build
index ed2a4048d18..57fe8c3eaec 100644
--- a/src/interfaces/libpq/meson.build
+++ b/src/interfaces/libpq/meson.build
@@ -1,8 +1,5 @@
# Copyright (c) 2022-2024, PostgreSQL Global Development Group
-# test/ is entered via top-level meson.build, that way it can use the default
-# args for executables (which depend on libpq).
-
libpq_sources = files(
'fe-auth-scram.c',
'fe-auth.c',
@@ -107,6 +104,7 @@ install_data('pg_service.conf.sample',
install_dir: dir_data,
)
+subdir('test')
tests += {
'name': 'libpq',
@@ -125,6 +123,7 @@ tests += {
'with_gssapi': gssapi.found() ? 'yes' : 'no',
'with_krb_srvnam': 'postgres',
},
+ 'deps': libpq_test_deps,
},
}
diff --git a/src/interfaces/libpq/test/meson.build b/src/interfaces/libpq/test/meson.build
index 21dd37f69bc..16c0c00b6bc 100644
--- a/src/interfaces/libpq/test/meson.build
+++ b/src/interfaces/libpq/test/meson.build
@@ -1,5 +1,7 @@
# Copyright (c) 2022-2024, PostgreSQL Global Development Group
+libpq_test_deps = []
+
libpq_uri_regress_sources = files(
'libpq_uri_regress.c',
)
@@ -10,9 +12,9 @@ if host_system == 'windows'
'--FILEDESC', 'libpq test program',])
endif
-testprep_targets += executable('libpq_uri_regress',
+libpq_test_deps += executable('libpq_uri_regress',
libpq_uri_regress_sources,
- dependencies: [frontend_code, libpq],
+ dependencies: [frontend_no_fe_utils_code, libpq],
kwargs: default_bin_args + {
'install': false,
}
@@ -29,10 +31,12 @@ if host_system == 'windows'
'--FILEDESC', 'libpq test program',])
endif
-testprep_targets += executable('libpq_testclient',
+libpq_test_deps += executable('libpq_testclient',
libpq_testclient_sources,
- dependencies: [frontend_code, libpq],
+ dependencies: [frontend_no_fe_utils_code, libpq],
kwargs: default_bin_args + {
'install': false,
}
)
+
+testprep_targets += libpq_test_deps
--
2.45.2.746.g06e570c0df.dirty
On 07.12.24 21:45, Andres Freund wrote:
If a test target does not have ay dependency 'meson test' treats it has having
a dependency on the project default test. Which in turn currently includes
everything that will be installed, i.e. everything that tmp_install might
need. But that's not something I think we should rely on going forward.
I don't understand this. What is the project default test? I don't
find any documentation about this default dependency. (Not everything
like that tends to be documented, but do you have a source code
reference or a GH issue?)
Hi,
On 2024-12-09 16:06:18 +0100, Peter Eisentraut wrote:
On 07.12.24 21:45, Andres Freund wrote:
If a test target does not have ay dependency 'meson test' treats it has having
a dependency on the project default test. Which in turn currently includes
everything that will be installed, i.e. everything that tmp_install might
need. But that's not something I think we should rely on going forward.I don't understand this. What is the project default test?
Oops, default target, not test.
I don't find any documentation about this default dependency. (Not
everything like that tends to be documented, but do you have a source code
reference or a GH issue?)
Greetings,
Andres Freund
Hi,
I pushed these changes as part of
/messages/by-id/fucdlk6bgvrz6vb6ruscxa5xof5w2c3voxoqontl7oasf4idxl@uyljndimefct
Greetings,
Andres Freund