pgsql: Check that we have a working tar before trying to use it
Check that we have a working tar before trying to use it
Issue exposed by commit edc2332550 and the buildfarm.
Backpatch to release 14 where this usage started.
Branch
------
master
Details
-------
https://git.postgresql.org/pg/commitdiff/f920f7e799c587228227ec94356c760e3f3d5f2b
Modified Files
--------------
src/bin/pg_basebackup/t/010_pg_basebackup.pl | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
Andrew Dunstan <andrew@dunslane.net> writes:
Check that we have a working tar before trying to use it
I don't like the way you did this:
+ || system_log($tar, '--version') != 0);
ISTM that effectively restricts the test to only running
on machines with GNU tar, which basically removes all the
interest of it. We know what GNU tar does ... it's the
weird legacy tar versions that might teach us something.
See 57b5a9646 for a recent example of the sort of bug
this test can no longer find.
regards, tom lane
On 12/8/21 10:39, Tom Lane wrote:
Andrew Dunstan <andrew@dunslane.net> writes:
Check that we have a working tar before trying to use it
I don't like the way you did this:
+ || system_log($tar, '--version') != 0);
ISTM that effectively restricts the test to only running
on machines with GNU tar, which basically removes all the
interest of it. We know what GNU tar does ... it's the
weird legacy tar versions that might teach us something.
See 57b5a9646 for a recent example of the sort of bug
this test can no longer find.
I tested on a freebsd system before I did this for that reason. It's not
using GNU tar:
user@freebsd:~ $ tar --version
bsdtar 3.5.1 - libarchive 3.5.1 zlib/1.2.11 liblzma/5.2.5 bz2lib/1.0.8
Do you have an alternative test we could use? This is almost exactly
what we use to test for lz4.
cheers
andrew
--
Andrew Dunstan
EDB: https://www.enterprisedb.com
Andrew Dunstan <andrew@dunslane.net> writes:
On 12/8/21 10:39, Tom Lane wrote:
ISTM that effectively restricts the test to only running
on machines with GNU tar, which basically removes all the
interest of it. We know what GNU tar does ... it's the
weird legacy tar versions that might teach us something.
See 57b5a9646 for a recent example of the sort of bug
this test can no longer find.
I tested on a freebsd system before I did this for that reason. It's not
using GNU tar:
user@freebsd:~ $ tar --version
bsdtar 3.5.1 - libarchive 3.5.1 zlib/1.2.11 liblzma/5.2.5 bz2lib/1.0.8
Right, but on AIX:
tgl@gcc119:[/home/tgl]tar --version
tar: Not a recognized flag: -
Usage: tar -{c|r|t|u|x} [ -BdDEFhilmopRUsvwZ ] [ -Number ] [ -f TarFil e ]
[ -b Blocks ] [ -S [ Feet ] | [ Feet@Density ] | [ Blocksb ] ]
[ -L InputList ] [-X ExcludeFile] [ -N Blocks ] [ -C Directory ] File ...
Usage: tar {c|r|t|u|x} [ bBdDEfFhilLXmNopRsSUvwZ[0-9] ] ]
[ Blocks ] [ TarFile ] [ InputList ] [ ExcludeFile ]
[ [ Feet ] | [ Feet@Density ] | [ Blocksb ] ] [-C Directory ] File ...
tgl@gcc119:[/home/tgl]echo $?
1
Do you have an alternative test we could use?
I think you need to be straight up about it, say
touch foo; tar cf foo.tar foo
(At least on the Unix machines I tried, it works to use /dev/null
as the output file, saving one cleanup step. But I don't know
if that'll work on Windows.)
regards, tom lane
On 12/8/21 11:15, Tom Lane wrote:
Andrew Dunstan <andrew@dunslane.net> writes:
On 12/8/21 10:39, Tom Lane wrote:
ISTM that effectively restricts the test to only running
on machines with GNU tar, which basically removes all the
interest of it. We know what GNU tar does ... it's the
weird legacy tar versions that might teach us something.
See 57b5a9646 for a recent example of the sort of bug
this test can no longer find.I tested on a freebsd system before I did this for that reason. It's not
using GNU tar:
user@freebsd:~ $ tar --version
bsdtar 3.5.1 - libarchive 3.5.1 zlib/1.2.11 liblzma/5.2.5 bz2lib/1.0.8Right, but on AIX:
tgl@gcc119:[/home/tgl]tar --version
tar: Not a recognized flag: -
Usage: tar -{c|r|t|u|x} [ -BdDEFhilmopRUsvwZ ] [ -Number ] [ -f TarFil e ]
[ -b Blocks ] [ -S [ Feet ] | [ Feet@Density ] | [ Blocksb ] ]
[ -L InputList ] [-X ExcludeFile] [ -N Blocks ] [ -C Directory ] File ...
Usage: tar {c|r|t|u|x} [ bBdDEfFhilLXmNopRsSUvwZ[0-9] ] ]
[ Blocks ] [ TarFile ] [ InputList ] [ ExcludeFile ]
[ [ Feet ] | [ Feet@Density ] | [ Blocksb ] ] [-C Directory ] File ...
tgl@gcc119:[/home/tgl]echo $?
1Do you have an alternative test we could use?
I think you need to be straight up about it, say
touch foo; tar cf foo.tar foo
(At least on the Unix machines I tried, it works to use /dev/null
as the output file, saving one cleanup step. But I don't know
if that'll work on Windows.)
Fair enough.
Actually, I think it makes more sense to simply revert this. bowerbird,
like all buildfarm animals, has a working tar. It's just that this one
is apparently seriously confused by using paths with a drive spec.
cheers
andrew
--
Andrew Dunstan
EDB: https://www.enterprisedb.com
Andrew Dunstan <andrew@dunslane.net> writes:
Actually, I think it makes more sense to simply revert this. bowerbird,
like all buildfarm animals, has a working tar.
Right, it doesn't seem like tar is the biggest lift when setting up
a buildfarm critter.
It's just that this one
is apparently seriously confused by using paths with a drive spec.
Hm, so how would this test have handled that anyway?
regards, tom lane
On 12/8/21 12:05, Tom Lane wrote:
Andrew Dunstan <andrew@dunslane.net> writes:
Actually, I think it makes more sense to simply revert this. bowerbird,
like all buildfarm animals, has a working tar.Right, it doesn't seem like tar is the biggest lift when setting up
a buildfarm critter.It's just that this one
is apparently seriously confused by using paths with a drive spec.Hm, so how would this test have handled that anyway?
It would have passed the test and failed the invocation as before. It
turns out that the tar being used is from the (old) MsysGit
installation. So I'm rearranging the path so it finds the system tar
instead. That should take care of things. (testing first before I do
anything else). On my initial test the system tar handled paths with
drive specs just fine.
cheers
andrew
--
Andrew Dunstan
EDB: https://www.enterprisedb.com
Hi,
On 2021-12-08 12:53:29 -0500, Andrew Dunstan wrote:
It would have passed the test and failed the invocation as before. It
turns out that the tar being used is from the (old) MsysGit
installation. So I'm rearranging the path so it finds the system tar
instead. That should take care of things. (testing first before I do
anything else). On my initial test the system tar handled paths with
drive specs just fine.
I just hit this issue. The tar included in a just-updated git-for-windows
fails with "tar: Cannot connect to c: resolve failed".
Git appears to install itself in front of c:\windows\system32 in PATH (which
contains the system tar). Which I think means that after edc2332550 this will
have to be manually addressed by most people running tests on windows
manually? That doesn't seem great :(
Greetings,
Andres Freund
Andres Freund <andres@anarazel.de> writes:
Git appears to install itself in front of c:\windows\system32 in PATH (which
contains the system tar). Which I think means that after edc2332550 this will
have to be manually addressed by most people running tests on windows
manually? That doesn't seem great :(
Can we make the default be the system tar (ie a full path, not
just "tar")? I have no idea if that's a stable thing on Windows ...
regards, tom lane
Hi,
On 2021-12-08 21:18:54 -0500, Tom Lane wrote:
Andres Freund <andres@anarazel.de> writes:
Git appears to install itself in front of c:\windows\system32 in PATH (which
contains the system tar). Which I think means that after edc2332550 this will
have to be manually addressed by most people running tests on windows
manually? That doesn't seem great :(Can we make the default be the system tar (ie a full path, not
just "tar")? I have no idea if that's a stable thing on Windows ...
It's not stable, but afaict all the "instability" can be handled by relying on
a system-set environment variable... Referencing it as
$ENV{SYSTEMROOT}/system32/tar.exe ought to work? I'll test that approach via
CI, but of course that won't verify that it works in all kinds of odd
installations.
Greetings,
Andres Freund
Hi,
On 2021-12-08 18:46:04 -0800, Andres Freund wrote:
On 2021-12-08 21:18:54 -0500, Tom Lane wrote:
Andres Freund <andres@anarazel.de> writes:
Git appears to install itself in front of c:\windows\system32 in PATH (which
contains the system tar). Which I think means that after edc2332550 this will
have to be manually addressed by most people running tests on windows
manually? That doesn't seem great :(Can we make the default be the system tar (ie a full path, not
just "tar")? I have no idea if that's a stable thing on Windows ...It's not stable, but afaict all the "instability" can be handled by relying on
a system-set environment variable... Referencing it as
$ENV{SYSTEMROOT}/system32/tar.exe ought to work? I'll test that approach via
CI, but of course that won't verify that it works in all kinds of odd
installations.
That worked: https://cirrus-ci.com/task/6302355785252864
https://github.com/anarazel/postgres/commit/49cf19f160080353da391f177a7d8c28218023bb
[03:01:52.451] t/010_pg_basebackup.pl ... ok
[03:02:09.737] t/020_pg_receivewal.pl ... ok
[03:02:11.837] t/030_pg_recvlogical.pl .. ok
[03:02:11.848] Result: PASS
From the commit (which also removes a previous way to address this, ignore
that):
# For tar default to the tar shipped with windows. It's common to have git's
# tar in PATH, and that doesn't handle drive letters.
I guess the comma before and is wrong? But somehow it sounds odd to me to not
have it. I guess it should be a which?
Greetings,
Andres Freund
On Wed, Dec 08, 2021 at 07:08:56PM -0800, Andres Freund wrote:
On 2021-12-08 18:46:04 -0800, Andres Freund wrote:
On 2021-12-08 21:18:54 -0500, Tom Lane wrote:
It's not stable, but afaict all the "instability" can be handled by relying on
a system-set environment variable... Referencing it as
$ENV{SYSTEMROOT}/system32/tar.exe ought to work? I'll test that approach via
CI, but of course that won't verify that it works in all kinds of odd
installations.That worked: https://cirrus-ci.com/task/6302355785252864
https://github.com/anarazel/postgres/commit/49cf19f160080353da391f177a7d8c28218023bb
Well, I am afraid that this is not a completely stable path. That
would not work here, for one. Could it be possible to use a soft
failure instead? If $ENV{TAR} is defined but the command cannot be
found, what Tom proposed upthread seemed like a good alternative to
me, as of:
/messages/by-id/3657697.1638980149@sss.pgh.pa.us
If this is switched to a hard requirement for any Windows environment
(I can adapt to the new order), it would be better to document all
that in the section of the docs for Windows and the tests of
vcregress.pl or users could finish with surprising failures.
--
Michael
Hi,
On 2021-12-09 14:47:39 +0900, Michael Paquier wrote:
On Wed, Dec 08, 2021 at 07:08:56PM -0800, Andres Freund wrote:
On 2021-12-08 18:46:04 -0800, Andres Freund wrote:
On 2021-12-08 21:18:54 -0500, Tom Lane wrote:
It's not stable, but afaict all the "instability" can be handled by relying on
a system-set environment variable... Referencing it as
$ENV{SYSTEMROOT}/system32/tar.exe ought to work? I'll test that approach via
CI, but of course that won't verify that it works in all kinds of odd
installations.That worked: https://cirrus-ci.com/task/6302355785252864
https://github.com/anarazel/postgres/commit/49cf19f160080353da391f177a7d8c28218023bbWell, I am afraid that this is not a completely stable path. That
would not work here, for one.
Could you expand on why it doesn't work? As far as I can tell tar is shipped
with windows these days, and %SYSTEMROOT%/system32/tar.exe should point to
that tar?
Greetings,
Andres Freund
On Wed, Dec 08, 2021 at 10:49:28PM -0800, Andres Freund wrote:
Could you expand on why it doesn't work? As far as I can tell tar is shipped
with windows these days, and %SYSTEMROOT%/system32/tar.exe should point to
that tar?
FWIW, my Windows 10 VM, that I got from Microsoft a couple of years
ago, does not ship it in this location, simply, even after updates.
And I am pretty sure that you cannot assume that this will exist at
this location for all the Windows environments we still support the
compilation of PG. My thing runs Visual 2015 that we still support,
so, if possible, I'd like to keep it around a bit more as it is useful
to check and test incompatibilities and/or bug fixes, and it has
caught problems in the past. I got some copies of tar.exe lying
around, one from a git installation and a second one from Msys, but
both don't work even if $ENV{TAR} points at them :/
--
Michael
Hi,
On 2021-12-09 16:10:39 +0900, Michael Paquier wrote:
FWIW, my Windows 10 VM, that I got from Microsoft a couple of years
ago, does not ship it in this location, simply, even after updates.
That is odd - MS says it's available in all types of windows 10:
And I am pretty sure that you cannot assume that this will exist at
this location for all the Windows environments we still support the
compilation of PG.
I don't think we need to. I think it's important to optimize the long-term
effort (i.e. everyone using git needing to set TAR) => having a default that
works going forward is important. Needing to configure tar on old windows
installations (when running tap tests), isn't a comparable long-term effort.
My thing runs Visual 2015 that we still support, so, if possible, I'd like
to keep it around a bit more as it is useful to check and test
incompatibilities and/or bug fixes, and it has caught problems in the past.
IMO we should just drop support for that.
I got some copies of tar.exe lying around, one from a git installation and a
second one from Msys, but both don't work even if $ENV{TAR} points at them
:/ -- Michael
Well, in that case what I'm suggesting doesn't even hurt...
Greetings,
Andres Freund
Michael Paquier <michael@paquier.xyz> writes:
... I got some copies of tar.exe lying
around, one from a git installation and a second one from Msys, but
both don't work even if $ENV{TAR} points at them :/
What goes wrong exactly?
The test I proposed ("tar cf foo.tar foo") doesn't seem like it
will catch the actual problem, which IIUC is that these tars
don't like a drive letter in the file name(s). Can we arrange
to strip that in what's passed to $TAR ? If not, it seems like
we have to test for exactly that point (ie Windows-only test).
regards, tom lane
On 12/8/21 21:16, Andres Freund wrote:
Hi,
On 2021-12-08 12:53:29 -0500, Andrew Dunstan wrote:
It would have passed the test and failed the invocation as before. It
turns out that the tar being used is from the (old) MsysGit
installation. So I'm rearranging the path so it finds the system tar
instead. That should take care of things. (testing first before I do
anything else). On my initial test the system tar handled paths with
drive specs just fine.I just hit this issue. The tar included in a just-updated git-for-windows
fails with "tar: Cannot connect to c: resolve failed".Git appears to install itself in front of c:\windows\system32 in PATH (which
contains the system tar). Which I think means that after edc2332550 this will
have to be manually addressed by most people running tests on windows
manually? That doesn't seem great :(
I'm not sure how you're installing git, or even if you're installing it
at all - maybe it comes with the image you're using. In my standard
Windows VM setup I just install git via chocolatey, and it only puts
git's cmd directory in the path, not the bin directory. This is a git
install option I know I've used in the past when installing manually.
(The setup where bowerbird runs is much older).
cheers
andrew
--
Andrew Dunstan
EDB: https://www.enterprisedb.com
On 12/9/21 02:10, Michael Paquier wrote:
On Wed, Dec 08, 2021 at 10:49:28PM -0800, Andres Freund wrote:
Could you expand on why it doesn't work? As far as I can tell tar is shipped
with windows these days, and %SYSTEMROOT%/system32/tar.exe should point to
that tar?FWIW, my Windows 10 VM, that I got from Microsoft a couple of years
ago, does not ship it in this location, simply, even after updates.
That's odd. What edition of Windows 10 do you have? If it's not Pro I
could understand it. I just checked and every Windows 10 machine I have
here has it - and some of them are quite old.
cheers
andrew
--
Andrew Dunstan
EDB: https://www.enterprisedb.com
On Thu, Dec 09, 2021 at 09:05:21AM -0500, Andrew Dunstan wrote:
That's odd. What edition of Windows 10 do you have? If it's not Pro I
could understand it. I just checked and every Windows 10 machine I have
here has it - and some of them are quite old.
Digging into that, I cannot find an exact build number, but it is
named Windows 10 Enterprise evaluation, which is, as far as I recall,
a pre-created VM image for VMware, so that was pretty useful.
I have been testing a couple of ideas to figure out how I could solve
that and if a system update could add a proper tar command, but I have
not been able to fix my thing. For the time being, I think that I am
just going to stick a ENV{TAR} = '' in buildenv.pl to bypass the
issue, and I have no objections to switch to
%SYSTEMROOT%/system32/tar.exe by default. Anyway, it would be nice to
document LZ4, GZIP_PROGRAM and TAR and the defaults we rely on. Also,
the default values of LZ4, TAR and GZIP_PROGRAM had better be set
*before* loading buildenv.pl, and not after it is loaded, no? On
HEAD, one has no way to unset any of those variables so it is not
possible to skip things if an environment has in its PATH a funky
command that TAP would refuse, like a tar command from MinGW or the
git installation.
--
Michael
On 12/13/21 03:08, Michael Paquier wrote:
Also,
the default values of LZ4, TAR and GZIP_PROGRAM had better be set
*before* loading buildenv.pl, and not after it is loaded, no? On
HEAD, one has no way to unset any of those variables so it is not
possible to skip things if an environment has in its PATH a funky
command that TAP would refuse, like a tar command from MinGW or the
git installation.
No. The code only sets them if they have not been previously set by
buildenv.pl or the calling environment. That's what "||=" means.
cheers
andrew
--
Andrew Dunstan
EDB: https://www.enterprisedb.com