From: Justin Pryzby <pryzby@telsasoft.com>
To: Andrew Dunstan <andrew@dunslane.net>
Cc: Andres Freund <andres@anarazel.de>
Cc: Tom Lane <tgl@sss.pgh.pa.us>
Cc: Robert Haas <robertmhaas@gmail.com>
Cc: pgsql-hackers@postgresql.org, Thomas Munro <thomas.munro@gmail.com>
Cc: Melanie Plageman <melanieplageman@gmail.com>
Cc: Peter Eisentraut <peter.eisentraut@enterprisedb.com>
Cc: Daniel Gustafsson <daniel@yesql.se>
Subject: Re: set TESTDIR from perl rather than Makefile
Date: Thu, 24 Feb 2022 19:17:04 -0600
Message-ID: <20220225011704.GA9008@telsasoft.com> (raw)
In-Reply-To: <0d18be28-8314-6587-8ea2-713bb663d539@dunslane.net>
References: <20220219234148.GC9008@telsasoft.com>
<20220219235309.GA27807@telsasoft.com>
<0d18be28-8314-6587-8ea2-713bb663d539@dunslane.net>
On Mon, Feb 21, 2022 at 07:00:54AM -0500, Andrew Dunstan wrote:
> On 2/19/22 18:53, Justin Pryzby wrote:
> > On Sat, Feb 19, 2022 at 05:41:49PM -0600, Justin Pryzby wrote:
> >> I rebased and fixed the check-guc script to work, made it work with vpath
> >> builds, and cleaned it up some.
> > I also meant to also attach it.
>
> This is going to break a bunch of stuff as written.
>
> First, it's not doing the same thing. The current system sets TESTDIR to
> be the parent of the directory that holds the test. e.g. for
> src/bin/pg_ctl/t/001_start_stop.pl it's src/bin/pg_ctl in the build
> tree, not the 't' subdirectory. This patch apparently sets it to the 't'
> subdirectory. That will break anything that goes looking for log files
> in the current location, like the buildfarm client, and possibly some CI
> setups as well.
Yes, thanks.
I changed the patch to use ENV{CURDIR} || dirname(dirname($0)). If I'm not
wrong, that seems to be doing the right thing.
> Also, unless I'm mistaken it appears to to the wrong thing for vpath
> builds:
>
> my $test_dir = File::Spec->rel2abs(dirname($0));
>
> is completely wrong for vpaths, since that will place it in the source
> tree, not the build tree.
>
> Last, and for no explained reason that I can see, the patch undoes
> commit f4ce6c4d3a, but only for msvc builds. Even if that's justified it
> appears to have no relevance to this patch.
Andres' idea is that perl should set TESTDIR and PATH. Here I commented out
PATH, and had the improbable issue that nothing seemed to be breaking,
including the pipeline test under msvc. It'd be helpful to know what
configuration that breaks so I can test that it's broken and then test that
it's fixed when set from within perl...
I got busy here, and may not be able to progress this for awhile.
Attachments:
[text/x-diff] 0001-wip-set-TESTDIR-from-src-test-perl-rather-than-Makef.patch (4.1K, ../20220225011704.GA9008@telsasoft.com/2-0001-wip-set-TESTDIR-from-src-test-perl-rather-than-Makef.patch)
download | inline diff:
From b46b405565a2fce3f96a1dcddf6d35ae7f3acc6d Mon Sep 17 00:00:00 2001
From: Justin Pryzby <pryzbyj@telsasoft.com>
Date: Sat, 19 Feb 2022 13:06:52 -0600
Subject: [PATCH] wip: set TESTDIR from src/test/perl rather than
Makefile/vcregress
These seem most likely to break:
make check -C src/bin/psql
make check -C src/bin/pgbench
make check -C src/test/modules/test_misc
make check -C src/test/modules/libpq_pipeline
PROVE_TESTS=t/027_stream_regress.pl make check -C src/test/recovery
---
src/Makefile.global.in | 9 ++++++---
src/test/perl/PostgreSQL/Test/Utils.pm | 17 ++++++++++-------
src/tools/msvc/vcregress.pl | 4 +---
3 files changed, 17 insertions(+), 13 deletions(-)
diff --git a/src/Makefile.global.in b/src/Makefile.global.inindex c980444233..92649d0193 100644--- a/src/Makefile.global.in+++ b/src/Makefile.global.in@@ -451,8 +451,9 @@ define prove_installcheck
rm -rf '$(CURDIR)'/tmp_check
$(MKDIR_P) '$(CURDIR)'/tmp_check
cd $(srcdir) && \
- TESTDIR='$(CURDIR)' PATH="$(bindir):$(CURDIR):$$PATH" \+ PATH="$(bindir):$(CURDIR):$$PATH" \
PGPORT='6$(DEF_PGPORT)' top_builddir='$(CURDIR)/$(top_builddir)' \
+ PG_SUBDIR='$(CURDIR)' \
PG_REGRESS='$(CURDIR)/$(top_builddir)/src/test/regress/pg_regress' \
$(PROVE) $(PG_PROVE_FLAGS) $(PROVE_FLAGS) $(if $(PROVE_TESTS),$(PROVE_TESTS),t/*.pl)
endef
@@ -461,8 +462,9 @@ define prove_installcheck
rm -rf '$(CURDIR)'/tmp_check
$(MKDIR_P) '$(CURDIR)'/tmp_check
cd $(srcdir) && \
- TESTDIR='$(CURDIR)' PATH="$(bindir):$(CURDIR):$$PATH" \+ PATH="$(bindir):$(CURDIR):$$PATH" \
PGPORT='6$(DEF_PGPORT)' top_builddir='$(top_builddir)' \
+ PG_SUBDIR='$(CURDIR)' \
PG_REGRESS='$(top_builddir)/src/test/regress/pg_regress' \
$(PROVE) $(PG_PROVE_FLAGS) $(PROVE_FLAGS) $(if $(PROVE_TESTS),$(PROVE_TESTS),t/*.pl)
endef
@@ -472,7 +474,8 @@ define prove_check
rm -rf '$(CURDIR)'/tmp_check
$(MKDIR_P) '$(CURDIR)'/tmp_check
cd $(srcdir) && \
- TESTDIR='$(CURDIR)' $(with_temp_install) PGPORT='6$(DEF_PGPORT)' \+ $(with_temp_install) PGPORT='6$(DEF_PGPORT)' \+ PG_SUBDIR='$(CURDIR)' \
PG_REGRESS='$(CURDIR)/$(top_builddir)/src/test/regress/pg_regress' \
$(PROVE) $(PG_PROVE_FLAGS) $(PROVE_FLAGS) $(if $(PROVE_TESTS),$(PROVE_TESTS),t/*.pl)
endef
diff --git a/src/test/perl/PostgreSQL/Test/Utils.pm b/src/test/perl/PostgreSQL/Test/Utils.pmindex 46cd746796..74ccaa08d9 100644--- a/src/test/perl/PostgreSQL/Test/Utils.pm+++ b/src/test/perl/PostgreSQL/Test/Utils.pm@@ -184,19 +184,22 @@ INIT
# test may still fail, but it's more likely to report useful facts.
$SIG{PIPE} = 'IGNORE';
- # Determine output directories, and create them. The base path is the- # TESTDIR environment variable, which is normally set by the invoking- # Makefile.- $tmp_check = $ENV{TESTDIR} ? "$ENV{TESTDIR}/tmp_check" : "tmp_check";+ my $test_dir = File::Spec->rel2abs($ENV{PG_SUBDIR} || dirname(dirname($0)));++ my $test_name = basename($0);+ $test_name =~ s/\.[^.]+$//;++ # Determine output directories, and create them.+ # TODO: set srcdir?+ $tmp_check = "$test_dir/tmp_check";
$log_path = "$tmp_check/log";
+ $ENV{TESTDIR} = $test_dir;
mkdir $tmp_check;
mkdir $log_path;
# Open the test log file, whose name depends on the test name.
- $test_logfile = basename($0);- $test_logfile =~ s/\.[^.]+$//;- $test_logfile = "$log_path/regress_log_$test_logfile";+ $test_logfile = "$log_path/regress_log_$test_name";
open my $testlog, '>', $test_logfile
or die "could not open STDOUT to logfile \"$test_logfile\": $!";
diff --git a/src/tools/msvc/vcregress.pl b/src/tools/msvc/vcregress.plindex e2b0db0879..c1876fbdab 100644--- a/src/tools/msvc/vcregress.pl+++ b/src/tools/msvc/vcregress.pl@@ -261,10 +261,8 @@ sub tap_check
$ENV{PG_REGRESS} = "$topdir/$Config/pg_regress/pg_regress";
$ENV{REGRESS_SHLIB} = "$topdir/src/test/regress/regress.dll";
- $ENV{TESTDIR} = "$dir";
my $module = basename $dir;
- # add the module build dir as the second element in the PATH- $ENV{PATH} =~ s!;!;$topdir/$Config/$module;!;+ # $ENV{VCREGRESS_MODE} = $Config;
rmtree('tmp_check');
system(@args);
--
2.17.1
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Reply to all the recipients using the --to and --cc options:
reply via email
To: pgsql-hackers@postgresql.org
Cc: pryzby@telsasoft.com, andrew@dunslane.net, andres@anarazel.de, tgl@sss.pgh.pa.us, robertmhaas@gmail.com, thomas.munro@gmail.com, melanieplageman@gmail.com, peter.eisentraut@enterprisedb.com, daniel@yesql.se
Subject: Re: set TESTDIR from perl rather than Makefile
In-Reply-To: <20220225011704.GA9008@telsasoft.com>
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
This inbox is served by DDX for PostgreSQL; see mirroring instructions
for how to clone and mirror all data and code used for this inbox