I have looked at your branch at 0545eec895:

258f6dc0a7 Don't hardcode tmp_check/ as test directory for tap tests
8ecc33cf04 Split TESTDIR into TESTLOGDIR and TESTDATADIR

I think these patches are split up a bit incorrectly.  If you apply
the first patch by itself, then the output appears in tab_comp_dir/
directly under the source directory.  And then the second patch moves
it to tmp_check/tap_comp_dir/.  If there is an intent to apply these
patches separately somehow, this should be cleaned up.

I haven't checked the second patch in detail yet, but it looks like
the thought was that the first patch is about ready to go.

834a40e609 meson: prereq: Extend gendef.pl in preparation for meson

I'm not qualified to check that in detail, but it looks reasonable
enough to me.

See attached patch (0001) for a perlcritic fix.

97a0b096e8 meson: prereq: Add src/tools/gen_export.pl

This produces leading whitespace in the output files that at least on
darwin wasn't there before.  See attached patch (0002).  This should
be checked again on other platforms as well.

Other than that this looks good.  Attached is a small cosmetic patch (0003).

40e363b263 meson: prereq: Refactor PG_TEST_EXTRA logic in autoconf build

Since I last looked, this has been turned into a meson option.  Which
is probably the best solution.  But then we should probably make this
a configure option as well.  Otherwise, it could get a bit confusing.
For example, I just unset PG_TEST_EXTRA in my environment to test
something with the meson build, but I was unaware that meson captures
the value at setup time, so my unsetting had no effect.

In any case, maybe adjust the regular expressions to check for word
boundaries, to maintain the original "whitespace-separated"
specification.  For example,

    elsif ($ENV{PG_TEST_EXTRA} !~ /\bssl\b/)

e0a8387660 solaris: Use versioning scripts instead of -Bsymbolic

This looks like a good idea.  The documentation clearly states that
-Bsymbolic shouldn't be used, at least not in the way we have been
doing.  Might as well go ahead with this and give it a whirl on the
build farm.

0545eec895 meson: Add docs

We should think more about how to arrange the documentation.  We
probably don't want to copy-and-paste all the introductory and
requirements information.  I think we can make this initially much
briefer, like the Windows installation chapter.  For example, instead
of documenting each setup option again, just mention which ones exist
and then point (link) to the configure chapter for details.


I spent a bit of time with the test suites.  I think there is a
problem in that selecting a test suite directly, like

    meson test -C _build --suite recovery

doesn't update the tmp_install.  So if this is the first thing you run
after a build, everything will fail.  Also, if you run this later, the
tmp_install doesn't get updated, so you're not testing up-to-date
code.
From 2f25b48271bceb7aa1551e015b03fc20b9aff162 Mon Sep 17 00:00:00 2001
From: Peter Eisentraut <pe...@eisentraut.org>
Date: Wed, 24 Aug 2022 11:23:52 +0200
Subject: [PATCH 1/3] Fix for perlcritic

---
 src/tools/msvc/gendef.pl | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/src/tools/msvc/gendef.pl b/src/tools/msvc/gendef.pl
index f08268e781..cfbdef9007 100644
--- a/src/tools/msvc/gendef.pl
+++ b/src/tools/msvc/gendef.pl
@@ -174,7 +174,7 @@ sub usage
 {
        if (-d $in)
        {
-               push @files, <$in/*.obj>;
+               push @files, glob "$in/*.obj";
        }
        else
        {
-- 
2.37.1

From a64c90e756d6996b7d8d9d63d42e30c72d4ce098 Mon Sep 17 00:00:00 2001
From: Peter Eisentraut <pe...@eisentraut.org>
Date: Wed, 24 Aug 2022 11:24:22 +0200
Subject: [PATCH 2/3] Fix whitespace in output of gen_export.pl

---
 src/tools/gen_export.pl | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/src/tools/gen_export.pl b/src/tools/gen_export.pl
index 6a8b196ad8..1265564473 100644
--- a/src/tools/gen_export.pl
+++ b/src/tools/gen_export.pl
@@ -56,7 +56,7 @@
                }
                elsif ($format eq 'darwin')
                {
-                       print $output_handle "    _$1\n";
+                       print $output_handle "_$1\n";
                }
                elsif ($format eq 'gnu')
                {
-- 
2.37.1

From 5b79baf6bbf4fa62f153a5e96e97f8d5a6345821 Mon Sep 17 00:00:00 2001
From: Peter Eisentraut <pe...@eisentraut.org>
Date: Wed, 24 Aug 2022 11:24:36 +0200
Subject: [PATCH 3/3] Some Perl code simplification in gen_export.pl

---
 src/tools/gen_export.pl | 4 +---
 1 file changed, 1 insertion(+), 3 deletions(-)

diff --git a/src/tools/gen_export.pl b/src/tools/gen_export.pl
index 1265564473..727105ba08 100644
--- a/src/tools/gen_export.pl
+++ b/src/tools/gen_export.pl
@@ -48,7 +48,7 @@
        {
                # don't do anything with a comment
        }
-       elsif (/^([^\s]+)\s+([^\s]+)/)
+       elsif (/^(\S+)\s+(\S+)/)
        {
                if ($format eq 'aix')
                {
@@ -79,5 +79,3 @@
 };
 ";
 }
-
-exit(0);
-- 
2.37.1

Reply via email to