Date: Tue, 14 Dec 2021 14:17:25 +0900
From: Michael Paquier <michael@paquier.xyz>
To: Andrew Dunstan <andrew@dunslane.net>
Cc: Andres Freund <andres@anarazel.de>, Tom Lane <tgl@sss.pgh.pa.us>,
	pgsql-committers@lists.postgresql.org
Subject: Re: pgsql: Check that we have a working tar before trying to use it
Message-ID: <Ybgo5ZWgqeUeW79F@paquier.xyz>
References: <20211209021604.bv5tfr23ymh4u7x4@alap3.anarazel.de>
 <3707204.1639016334@sss.pgh.pa.us>
 <20211209024604.dpmudj4ax2hoi7cx@alap3.anarazel.de>
 <20211209030856.ibxewzpc5ph7ckkp@alap3.anarazel.de>
 <YbGYe483803il3X7@paquier.xyz>
 <20211209064928.7f5lp6hxvw2f2j6o@alap3.anarazel.de>
 <YbGr7ylYwE56l91B@paquier.xyz>
 <95787b2b-d483-0b5e-b24b-1986cff93596@dunslane.net>
 <Ybb/itFhY3FrsyzF@paquier.xyz>
 <190aad26-d04b-f2c6-c4a3-e184377f4c8c@dunslane.net>
MIME-Version: 1.0
Content-Type: multipart/signed; micalg=pgp-sha512;
	protocol="application/pgp-signature"; boundary="+ulaRtPMBk9Q8pEa"
Content-Disposition: inline
In-Reply-To: <190aad26-d04b-f2c6-c4a3-e184377f4c8c@dunslane.net>
Archived-At: 
 <https://www.postgresql.org/message-id/Ybgo5ZWgqeUeW79F%40paquier.xyz>
Precedence: bulk


--+ulaRtPMBk9Q8pEa
Content-Type: multipart/mixed; boundary="PNUkzkUMJnLVCWwp"
Content-Disposition: inline


--PNUkzkUMJnLVCWwp
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline

On Mon, Dec 13, 2021 at 11:47:36AM -0500, Andrew Dunstan wrote:
> No. The code only sets them if they have not been previously set by
> buildenv.pl or the calling environment. That's what "||=" means.

Well, using ||= after the fact means that it is not possible to delete
any of those environment variables either in buildenv.pl, nor is it
possible to set them to empty string values, as vcregress.pl would
just reset them to the default value.  And that's the case I am
arguing for here.  We don't have any need to drop those default values
at all.  What I am arguing for here is the possibility to have
something more flexible than what HEAD proposes.

So, if we do something like say the attached, then it is possible to
handle environments like mine, while keeping the flexibility you are
looking for to set those defaults.  So that would be the best of both
worlds, no?.  I also think that we had better document all that
properly, as we do for any other TAP-related variable that can be used
in vcregress.pl.  All that leads me to the attached.
--
Michael

--PNUkzkUMJnLVCWwp
Content-Type: text/x-diff; charset=us-ascii
Content-Disposition: attachment; filename="win32-perl.patch"
Content-Transfer-Encoding: quoted-printable

diff --git a/doc/src/sgml/install-windows.sgml b/doc/src/sgml/install-windo=
ws.sgml
index b33359d3af..30dd0c7f75 100644
--- a/doc/src/sgml/install-windows.sgml
+++ b/doc/src/sgml/install-windows.sgml
@@ -517,6 +517,40 @@ $ENV{PROVE_FLAGS}=3D'--timer --jobs 2'
 $ENV{PROVE_TESTS}=3D't/020*.pl t/010*.pl'
 </programlisting>
   </para>
+
+  <para>
+   Some of the TAP tests depend on a set of external commands that would
+   optionally trigger tests related to them. Each one of those variables
+   can be set or unset in <filename>buildenv.pl</filename>:
+   <variablelist>
+    <varlistentry>
+     <term><varname>GZIP_PROGRAM</varname></term>
+     <listitem><para>
+       Path to a <application>gzip</application> command. The default is
+       <literal>gzip</literal>, that would be the command found in
+      <varname>PATH</varname>.
+     </para></listitem>
+    </varlistentry>
+
+    <varlistentry>
+     <term><varname>LZ4</varname></term>
+     <listitem><para>
+      Path to a <application>lz4</application> command. The default is
+      <literal>lz4</literal>, that would be the command found in
+      <varname>PATH</varname>.
+     </para></listitem>
+    </varlistentry>
+
+    <varlistentry>
+     <term><varname>TAR</varname></term>
+     <listitem><para>
+      Path to a <application>tar</application> command. The default is
+      <literal>tar</literal>, that would be the command found in
+      <varname>PATH</varname>.
+     </para></listitem>
+    </varlistentry>
+   </variablelist>
+  </para>
  </sect2>
=20
  </sect1>
diff --git a/src/tools/msvc/vcregress.pl b/src/tools/msvc/vcregress.pl
index 5975e7c9cd..8775da4edf 100644
--- a/src/tools/msvc/vcregress.pl
+++ b/src/tools/msvc/vcregress.pl
@@ -30,6 +30,13 @@ my $tmp_installdir =3D "$topdir/tmp_install";
 do './src/tools/msvc/config_default.pl';
 do './src/tools/msvc/config.pl' if (-f 'src/tools/msvc/config.pl');
=20
+# These values are defaults that can be overridden by the calling environm=
ent
+# (see buildenv.pl processing below).
+# c.f. src/Makefile.global.in and configure.ac
+$ENV{TAR} ||=3D 'tar';
+$ENV{LZ4} ||=3D 'lz4';
+$ENV{GZIP_PROGRAM} ||=3D 'gzip';
+
 # buildenv.pl is for specifying the build environment settings
 # it should contain lines like:
 # $ENV{PATH} =3D "c:/path/to/bison/bin;$ENV{PATH}";
@@ -67,13 +74,6 @@ $ENV{with_gssapi} =3D $config->{gss} ? 'yes' : 'no';
 $ENV{with_krb_srvnam} =3D $config->{krb_srvnam} || 'postgres';
 $ENV{with_readline} =3D 'no';
=20
-# These values are defaults that can be overridden by the calling environm=
ent
-# (see buildenv.pl processing above).
-# c.f. src/Makefile.global.in and configure.ac
-$ENV{TAR} ||=3D 'tar';
-$ENV{LZ4} ||=3D 'lz4';
-$ENV{GZIP_PROGRAM} ||=3D 'gzip';
-
 $ENV{PATH} =3D "$topdir/$Config/libpq;$ENV{PATH}";
=20
 if ($ENV{PERL5LIB})

--PNUkzkUMJnLVCWwp--

--+ulaRtPMBk9Q8pEa
Content-Type: application/pgp-signature; name="signature.asc"

-----BEGIN PGP SIGNATURE-----

iQIzBAABCgAdFiEEG72nH6vTowiyblFKnvQgOdbyQH0FAmG4KOUACgkQnvQgOdby
QH1ESw/+IS8qhpoiEquQbU5MYfcDQxeNi25a8Omz2tDYSxTgey9LmEj373NwwaeR
h6CSeNDdFpFVf0S80Cc6Ij7vYtMNievwZOfBhVzzRGBdTjXCBxl0apo7sOzEFZvz
JZWPGfYE/66KukifjNNgXQs1r9BbSsrVP4mhZZOHdfGqo04ToGZcbb0yeE7gCRyF
fsSlYxMIJ+J9I+68YHldUmwp3Iswn8K0uevYYlOp1q34jfI1ncMTVO2lVnYzsPvB
ssWt+5Y6piVEh5LUG0JVDUqoFb12whUHUMAjzNHjtgYKURWf+HRPmzycjfrbkQsD
VQro4iyBStbJX0wFPbi/iEvLVjg+SZydcngz/1Vu2vuVD42tBOKiEeeNfu2JAag9
9wiQ4/YYE7pxXUOvmyCihPRY9OCrAwAej0iq3Qm2TjpkDMk4U0/RDiiVheq7M+QD
KFPW+FhXT5n4nR37bVPDKAQcLnsjPna2N9LHDMCBRjHv9LAcQyUONO/KXjGwKXkw
MMUmHbi4OZ1Iy7psjU8UKBRINRYiwSt4Zt7GaaeokLl4SDSvXTR5+rn+foPSJx9T
47XFVF1NF5/fCCxEdd+T+kOpAijR0XtN8nCfO/b7xs7MiqN/p/Z1IUSd50fZMT9I
oxb4siF7BZ+jTmfJ4u4/pXUrXYNRlLyBoxQWvhttMUvf5rFJrJU=
=jzaQ
-----END PGP SIGNATURE-----

--+ulaRtPMBk9Q8pEa--