pg.ddx.io pgsql-committers@postgresql.org mailing list archive
help / color / mirror / Atom feedpgsql: Check that we have a working tar before trying to use it
25+ messages / 4 participants
[nested] [flat]
* pgsql: Check that we have a working tar before trying to use it
@ 2021-12-08 15:24 Andrew Dunstan <andrew@dunslane.net>
0 siblings, 0 replies; 25+ messages in thread
From: Andrew Dunstan @ 2021-12-08 15:24 UTC (permalink / raw)
To: pgsql-committers@lists.postgresql.org
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
------
REL_14_STABLE
Details
-------
https://git.postgresql.org/pg/commitdiff/90c08ed11332b766769b2e5ba114fb7ecb34e401
Modified Files
--------------
src/bin/pg_basebackup/t/010_pg_basebackup.pl | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
^ permalink raw reply [nested|flat] 25+ messages in thread
* pgsql: Check that we have a working tar before trying to use it
@ 2021-12-08 15:24 Andrew Dunstan <andrew@dunslane.net>
0 siblings, 1 reply; 25+ messages in thread
From: Andrew Dunstan @ 2021-12-08 15:24 UTC (permalink / raw)
To: pgsql-committers@lists.postgresql.org
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(-)
^ permalink raw reply [nested|flat] 25+ messages in thread
* Re: pgsql: Check that we have a working tar before trying to use it
@ 2021-12-08 15:39 Tom Lane <tgl@sss.pgh.pa.us>
parent: Andrew Dunstan <andrew@dunslane.net>
0 siblings, 1 reply; 25+ messages in thread
From: Tom Lane @ 2021-12-08 15:39 UTC (permalink / raw)
To: Andrew Dunstan <andrew@dunslane.net>; +Cc: pgsql-committers@lists.postgresql.org
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
^ permalink raw reply [nested|flat] 25+ messages in thread
* Re: pgsql: Check that we have a working tar before trying to use it
@ 2021-12-08 15:56 Andrew Dunstan <andrew@dunslane.net>
parent: Tom Lane <tgl@sss.pgh.pa.us>
0 siblings, 1 reply; 25+ messages in thread
From: Andrew Dunstan @ 2021-12-08 15:56 UTC (permalink / raw)
To: Tom Lane <tgl@sss.pgh.pa.us>; +Cc: pgsql-committers@lists.postgresql.org
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
^ permalink raw reply [nested|flat] 25+ messages in thread
* Re: pgsql: Check that we have a working tar before trying to use it
@ 2021-12-08 16:15 Tom Lane <tgl@sss.pgh.pa.us>
parent: Andrew Dunstan <andrew@dunslane.net>
0 siblings, 1 reply; 25+ messages in thread
From: Tom Lane @ 2021-12-08 16:15 UTC (permalink / raw)
To: Andrew Dunstan <andrew@dunslane.net>; +Cc: pgsql-committers@lists.postgresql.org
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
^ permalink raw reply [nested|flat] 25+ messages in thread
* Re: pgsql: Check that we have a working tar before trying to use it
@ 2021-12-08 16:53 Andrew Dunstan <andrew@dunslane.net>
parent: Tom Lane <tgl@sss.pgh.pa.us>
0 siblings, 1 reply; 25+ messages in thread
From: Andrew Dunstan @ 2021-12-08 16:53 UTC (permalink / raw)
To: Tom Lane <tgl@sss.pgh.pa.us>; +Cc: pgsql-committers@lists.postgresql.org
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.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.)
>
>
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
^ permalink raw reply [nested|flat] 25+ messages in thread
* Re: pgsql: Check that we have a working tar before trying to use it
@ 2021-12-08 17:05 Tom Lane <tgl@sss.pgh.pa.us>
parent: Andrew Dunstan <andrew@dunslane.net>
0 siblings, 1 reply; 25+ messages in thread
From: Tom Lane @ 2021-12-08 17:05 UTC (permalink / raw)
To: Andrew Dunstan <andrew@dunslane.net>; +Cc: pgsql-committers@lists.postgresql.org
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
^ permalink raw reply [nested|flat] 25+ messages in thread
* Re: pgsql: Check that we have a working tar before trying to use it
@ 2021-12-08 17:53 Andrew Dunstan <andrew@dunslane.net>
parent: Tom Lane <tgl@sss.pgh.pa.us>
0 siblings, 1 reply; 25+ messages in thread
From: Andrew Dunstan @ 2021-12-08 17:53 UTC (permalink / raw)
To: Tom Lane <tgl@sss.pgh.pa.us>; +Cc: pgsql-committers@lists.postgresql.org
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
^ permalink raw reply [nested|flat] 25+ messages in thread
* Re: pgsql: Check that we have a working tar before trying to use it
@ 2021-12-09 02:16 Andres Freund <andres@anarazel.de>
parent: Andrew Dunstan <andrew@dunslane.net>
0 siblings, 2 replies; 25+ messages in thread
From: Andres Freund @ 2021-12-09 02:16 UTC (permalink / raw)
To: Andrew Dunstan <andrew@dunslane.net>; +Cc: Tom Lane <tgl@sss.pgh.pa.us>; pgsql-committers@lists.postgresql.org
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
^ permalink raw reply [nested|flat] 25+ messages in thread
* Re: pgsql: Check that we have a working tar before trying to use it
@ 2021-12-09 02:18 Tom Lane <tgl@sss.pgh.pa.us>
parent: Andres Freund <andres@anarazel.de>
1 sibling, 1 reply; 25+ messages in thread
From: Tom Lane @ 2021-12-09 02:18 UTC (permalink / raw)
To: Andres Freund <andres@anarazel.de>; +Cc: Andrew Dunstan <andrew@dunslane.net>; pgsql-committers@lists.postgresql.org
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
^ permalink raw reply [nested|flat] 25+ messages in thread
* Re: pgsql: Check that we have a working tar before trying to use it
@ 2021-12-09 02:46 Andres Freund <andres@anarazel.de>
parent: Tom Lane <tgl@sss.pgh.pa.us>
0 siblings, 1 reply; 25+ messages in thread
From: Andres Freund @ 2021-12-09 02:46 UTC (permalink / raw)
To: Tom Lane <tgl@sss.pgh.pa.us>; +Cc: Andrew Dunstan <andrew@dunslane.net>; pgsql-committers@lists.postgresql.org
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
^ permalink raw reply [nested|flat] 25+ messages in thread
* Re: pgsql: Check that we have a working tar before trying to use it
@ 2021-12-09 03:08 Andres Freund <andres@anarazel.de>
parent: Andres Freund <andres@anarazel.de>
0 siblings, 1 reply; 25+ messages in thread
From: Andres Freund @ 2021-12-09 03:08 UTC (permalink / raw)
To: Tom Lane <tgl@sss.pgh.pa.us>; +Cc: Andrew Dunstan <andrew@dunslane.net>; pgsql-committers@lists.postgresql.org
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
^ permalink raw reply [nested|flat] 25+ messages in thread
* Re: pgsql: Check that we have a working tar before trying to use it
@ 2021-12-09 05:47 Michael Paquier <michael@paquier.xyz>
parent: Andres Freund <andres@anarazel.de>
0 siblings, 1 reply; 25+ messages in thread
From: Michael Paquier @ 2021-12-09 05:47 UTC (permalink / raw)
To: Andres Freund <andres@anarazel.de>; +Cc: Tom Lane <tgl@sss.pgh.pa.us>; Andrew Dunstan <andrew@dunslane.net>; pgsql-committers@lists.postgresql.org
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:
https://www.postgresql.org/message-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
Attachments:
[application/pgp-signature] signature.asc (832B, ../../YbGYe483803il3X7@paquier.xyz/2-signature.asc)
download
^ permalink raw reply [nested|flat] 25+ messages in thread
* Re: pgsql: Check that we have a working tar before trying to use it
@ 2021-12-09 06:49 Andres Freund <andres@anarazel.de>
parent: Michael Paquier <michael@paquier.xyz>
0 siblings, 1 reply; 25+ messages in thread
From: Andres Freund @ 2021-12-09 06:49 UTC (permalink / raw)
To: Michael Paquier <michael@paquier.xyz>; +Cc: Tom Lane <tgl@sss.pgh.pa.us>; Andrew Dunstan <andrew@dunslane.net>; pgsql-committers@lists.postgresql.org
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/49cf19f160080353da391f177a7d8c28218023bb
>
> Well, 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
^ permalink raw reply [nested|flat] 25+ messages in thread
* Re: pgsql: Check that we have a working tar before trying to use it
@ 2021-12-09 07:10 Michael Paquier <michael@paquier.xyz>
parent: Andres Freund <andres@anarazel.de>
0 siblings, 3 replies; 25+ messages in thread
From: Michael Paquier @ 2021-12-09 07:10 UTC (permalink / raw)
To: Andres Freund <andres@anarazel.de>; +Cc: Tom Lane <tgl@sss.pgh.pa.us>; Andrew Dunstan <andrew@dunslane.net>; pgsql-committers@lists.postgresql.org
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
Attachments:
[application/pgp-signature] signature.asc (832B, ../../YbGr7ylYwE56l91B@paquier.xyz/2-signature.asc)
download
^ permalink raw reply [nested|flat] 25+ messages in thread
* Re: pgsql: Check that we have a working tar before trying to use it
@ 2021-12-09 07:25 Andres Freund <andres@anarazel.de>
parent: Michael Paquier <michael@paquier.xyz>
2 siblings, 0 replies; 25+ messages in thread
From: Andres Freund @ 2021-12-09 07:25 UTC (permalink / raw)
To: Michael Paquier <michael@paquier.xyz>; +Cc: Tom Lane <tgl@sss.pgh.pa.us>; Andrew Dunstan <andrew@dunslane.net>; pgsql-committers@lists.postgresql.org
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:
https://docs.microsoft.com/en-us/virtualization/community/team-blog/2017/20171219-tar-and-curl-come-...
> 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
^ permalink raw reply [nested|flat] 25+ messages in thread
* Re: pgsql: Check that we have a working tar before trying to use it
@ 2021-12-09 13:45 Tom Lane <tgl@sss.pgh.pa.us>
parent: Michael Paquier <michael@paquier.xyz>
2 siblings, 0 replies; 25+ messages in thread
From: Tom Lane @ 2021-12-09 13:45 UTC (permalink / raw)
To: Michael Paquier <michael@paquier.xyz>; +Cc: Andres Freund <andres@anarazel.de>; Andrew Dunstan <andrew@dunslane.net>; pgsql-committers@lists.postgresql.org
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
^ permalink raw reply [nested|flat] 25+ messages in thread
* Re: pgsql: Check that we have a working tar before trying to use it
@ 2021-12-09 13:59 Andrew Dunstan <andrew@dunslane.net>
parent: Andres Freund <andres@anarazel.de>
1 sibling, 0 replies; 25+ messages in thread
From: Andrew Dunstan @ 2021-12-09 13:59 UTC (permalink / raw)
To: Andres Freund <andres@anarazel.de>; +Cc: Tom Lane <tgl@sss.pgh.pa.us>; pgsql-committers@lists.postgresql.org
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
^ permalink raw reply [nested|flat] 25+ messages in thread
* Re: pgsql: Check that we have a working tar before trying to use it
@ 2021-12-09 14:05 Andrew Dunstan <andrew@dunslane.net>
parent: Michael Paquier <michael@paquier.xyz>
2 siblings, 1 reply; 25+ messages in thread
From: Andrew Dunstan @ 2021-12-09 14:05 UTC (permalink / raw)
To: Michael Paquier <michael@paquier.xyz>; Andres Freund <andres@anarazel.de>; +Cc: Tom Lane <tgl@sss.pgh.pa.us>; pgsql-committers@lists.postgresql.org
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
^ permalink raw reply [nested|flat] 25+ messages in thread
* Re: pgsql: Check that we have a working tar before trying to use it
@ 2021-12-13 08:08 Michael Paquier <michael@paquier.xyz>
parent: Andrew Dunstan <andrew@dunslane.net>
0 siblings, 1 reply; 25+ messages in thread
From: Michael Paquier @ 2021-12-13 08:08 UTC (permalink / raw)
To: Andrew Dunstan <andrew@dunslane.net>; +Cc: Andres Freund <andres@anarazel.de>; Tom Lane <tgl@sss.pgh.pa.us>; pgsql-committers@lists.postgresql.org
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
Attachments:
[application/pgp-signature] signature.asc (832B, ../../Ybb%2FitFhY3FrsyzF@paquier.xyz/2-signature.asc)
download
^ permalink raw reply [nested|flat] 25+ messages in thread
* Re: pgsql: Check that we have a working tar before trying to use it
@ 2021-12-13 16:47 Andrew Dunstan <andrew@dunslane.net>
parent: Michael Paquier <michael@paquier.xyz>
0 siblings, 2 replies; 25+ messages in thread
From: Andrew Dunstan @ 2021-12-13 16:47 UTC (permalink / raw)
To: Michael Paquier <michael@paquier.xyz>; +Cc: Andres Freund <andres@anarazel.de>; Tom Lane <tgl@sss.pgh.pa.us>; pgsql-committers@lists.postgresql.org
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
^ permalink raw reply [nested|flat] 25+ messages in thread
* Re: pgsql: Check that we have a working tar before trying to use it
@ 2021-12-13 17:08 Andrew Dunstan <andrew@dunslane.net>
parent: Andrew Dunstan <andrew@dunslane.net>
1 sibling, 0 replies; 25+ messages in thread
From: Andrew Dunstan @ 2021-12-13 17:08 UTC (permalink / raw)
To: Michael Paquier <michael@paquier.xyz>; +Cc: Andres Freund <andres@anarazel.de>; Tom Lane <tgl@sss.pgh.pa.us>; pgsql-committers@lists.postgresql.org
On 12/13/21 11:47, Andrew Dunstan wrote:
> 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.
>
>
Sorry, I might have misunderstood what you meant here. Yes, you might
have a funky tar in your path. But then so you might with a Makefile
build, no? I'm not sure why MSVC builds should be different.
cheers
andrew
--
Andrew Dunstan
EDB: https://www.enterprisedb.com
^ permalink raw reply [nested|flat] 25+ messages in thread
* Re: pgsql: Check that we have a working tar before trying to use it
@ 2021-12-14 05:17 Michael Paquier <michael@paquier.xyz>
parent: Andrew Dunstan <andrew@dunslane.net>
1 sibling, 1 reply; 25+ messages in thread
From: Michael Paquier @ 2021-12-14 05:17 UTC (permalink / raw)
To: Andrew Dunstan <andrew@dunslane.net>; +Cc: Andres Freund <andres@anarazel.de>; Tom Lane <tgl@sss.pgh.pa.us>; pgsql-committers@lists.postgresql.org
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
Attachments:
[text/x-diff] win32-perl.patch (2.7K, ../../Ybgo5ZWgqeUeW79F@paquier.xyz/2-win32-perl.patch)
download | inline diff:
diff --git a/doc/src/sgml/install-windows.sgml b/doc/src/sgml/install-windows.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}='--timer --jobs 2'
$ENV{PROVE_TESTS}='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>
</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 = "$topdir/tmp_install";
do './src/tools/msvc/config_default.pl';
do './src/tools/msvc/config.pl' if (-f 'src/tools/msvc/config.pl');
+# These values are defaults that can be overridden by the calling environment
+# (see buildenv.pl processing below).
+# c.f. src/Makefile.global.in and configure.ac
+$ENV{TAR} ||= 'tar';
+$ENV{LZ4} ||= 'lz4';
+$ENV{GZIP_PROGRAM} ||= 'gzip';
+
# buildenv.pl is for specifying the build environment settings
# it should contain lines like:
# $ENV{PATH} = "c:/path/to/bison/bin;$ENV{PATH}";
@@ -67,13 +74,6 @@ $ENV{with_gssapi} = $config->{gss} ? 'yes' : 'no';
$ENV{with_krb_srvnam} = $config->{krb_srvnam} || 'postgres';
$ENV{with_readline} = 'no';
-# These values are defaults that can be overridden by the calling environment
-# (see buildenv.pl processing above).
-# c.f. src/Makefile.global.in and configure.ac
-$ENV{TAR} ||= 'tar';
-$ENV{LZ4} ||= 'lz4';
-$ENV{GZIP_PROGRAM} ||= 'gzip';
-
$ENV{PATH} = "$topdir/$Config/libpq;$ENV{PATH}";
if ($ENV{PERL5LIB})
[application/pgp-signature] signature.asc (832B, ../../Ybgo5ZWgqeUeW79F@paquier.xyz/3-signature.asc)
download
^ permalink raw reply [nested|flat] 25+ messages in thread
* Re: pgsql: Check that we have a working tar before trying to use it
@ 2021-12-14 14:14 Andrew Dunstan <andrew@dunslane.net>
parent: Michael Paquier <michael@paquier.xyz>
0 siblings, 1 reply; 25+ messages in thread
From: Andrew Dunstan @ 2021-12-14 14:14 UTC (permalink / raw)
To: Michael Paquier <michael@paquier.xyz>; +Cc: Andres Freund <andres@anarazel.de>; Tom Lane <tgl@sss.pgh.pa.us>; pgsql-committers@lists.postgresql.org
On 12/14/21 00:17, Michael Paquier wrote:
> 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.
Ok, if you think it's important then do it.
cheers
andrew
--
Andrew Dunstan
EDB: https://www.enterprisedb.com
^ permalink raw reply [nested|flat] 25+ messages in thread
* Re: pgsql: Check that we have a working tar before trying to use it
@ 2021-12-15 01:45 Michael Paquier <michael@paquier.xyz>
parent: Andrew Dunstan <andrew@dunslane.net>
0 siblings, 0 replies; 25+ messages in thread
From: Michael Paquier @ 2021-12-15 01:45 UTC (permalink / raw)
To: Andrew Dunstan <andrew@dunslane.net>; +Cc: Andres Freund <andres@anarazel.de>; Tom Lane <tgl@sss.pgh.pa.us>; pgsql-committers@lists.postgresql.org
On Tue, Dec 14, 2021 at 09:14:08AM -0500, Andrew Dunstan wrote:
> Ok, if you think it's important then do it.
Thanks. Applied down to 10, then, with the addition for the docs.
--
Michael
Attachments:
[application/pgp-signature] signature.asc (832B, ../../YblIyeD+FKkp0Wrf@paquier.xyz/2-signature.asc)
download
^ permalink raw reply [nested|flat] 25+ messages in thread
end of thread, other threads:[~2021-12-15 01:45 UTC | newest]
Thread overview: 25+ messages (download: mbox mbox.gz follow: Atom feed)
-- links below jump to the message on this page --
2021-12-08 15:24 pgsql: Check that we have a working tar before trying to use it Andrew Dunstan <andrew@dunslane.net>
2021-12-08 15:39 ` Tom Lane <tgl@sss.pgh.pa.us>
2021-12-08 15:56 ` Andrew Dunstan <andrew@dunslane.net>
2021-12-08 16:15 ` Tom Lane <tgl@sss.pgh.pa.us>
2021-12-08 16:53 ` Andrew Dunstan <andrew@dunslane.net>
2021-12-08 17:05 ` Tom Lane <tgl@sss.pgh.pa.us>
2021-12-08 17:53 ` Andrew Dunstan <andrew@dunslane.net>
2021-12-09 02:16 ` Andres Freund <andres@anarazel.de>
2021-12-09 02:18 ` Tom Lane <tgl@sss.pgh.pa.us>
2021-12-09 02:46 ` Andres Freund <andres@anarazel.de>
2021-12-09 03:08 ` Andres Freund <andres@anarazel.de>
2021-12-09 05:47 ` Michael Paquier <michael@paquier.xyz>
2021-12-09 06:49 ` Andres Freund <andres@anarazel.de>
2021-12-09 07:10 ` Michael Paquier <michael@paquier.xyz>
2021-12-09 07:25 ` Andres Freund <andres@anarazel.de>
2021-12-09 13:45 ` Tom Lane <tgl@sss.pgh.pa.us>
2021-12-09 14:05 ` Andrew Dunstan <andrew@dunslane.net>
2021-12-13 08:08 ` Michael Paquier <michael@paquier.xyz>
2021-12-13 16:47 ` Andrew Dunstan <andrew@dunslane.net>
2021-12-13 17:08 ` Andrew Dunstan <andrew@dunslane.net>
2021-12-14 05:17 ` Michael Paquier <michael@paquier.xyz>
2021-12-14 14:14 ` Andrew Dunstan <andrew@dunslane.net>
2021-12-15 01:45 ` Michael Paquier <michael@paquier.xyz>
2021-12-09 13:59 ` Andrew Dunstan <andrew@dunslane.net>
2021-12-08 15:24 pgsql: Check that we have a working tar before trying to use it Andrew Dunstan <andrew@dunslane.net>
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