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
>From dff957eb1b5f0c45e552affede99b3585bbcca00 Mon Sep 17 00:00:00 2001
From: Andres Freund <[email protected]>
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
>From fdf058d1b9255ee76dfa7129d4e7d9c4e9a726b6 Mon Sep 17 00:00:00 2001
From: Andres Freund <[email protected]>
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/[email protected]
---
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
>From d6e6e0a23638d7860545c9ff2bf0ccd752885e83 Mon Sep 17 00:00:00 2001
From: Andres Freund <[email protected]>
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
>From 88c0e3f9f6ea1f1aad4c5de4436ebde40a9c4c7e Mon Sep 17 00:00:00 2001
From: Andres Freund <[email protected]>
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 <[email protected]>
Author: Andres Freund <[email protected]>
Discussion: https://postgr.es/m/[email protected]
---
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
>From 23e9200b266093cd65f895b647af76b867d71d86 Mon Sep 17 00:00:00 2001
From: Andres Freund <[email protected]>
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
>From 14aefeae08975198abc73a073b15559e73ac3774 Mon Sep 17 00:00:00 2001
From: Andres Freund <[email protected]>
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
