From: | Andrew Dunstan <andrew(dot)dunstan(at)2ndquadrant(dot)com> |
---|---|
To: | Peter Eisentraut <peter(dot)eisentraut(at)2ndquadrant(dot)com>, Michael Paquier <michael(at)paquier(dot)xyz> |
Cc: | Juan José Santamaría Flecha <juanjo(dot)santamaria(at)gmail(dot)com>, Dagfinn Ilmari Mannsåker <ilmari(at)ilmari(dot)org>, pgsql-hackers <pgsql-hackers(at)postgresql(dot)org> |
Subject: | Re: TAP tests and symlinks on Windows |
Date: | 2020-07-08 13:54:35 |
Message-ID: | 19483ac1-0f31-cfd3-8ef7-5da0ac5a5f46@2ndQuadrant.com |
Views: | Raw Message | Whole Thread | Download mbox | Resend email |
Thread: | |
Lists: | pgsql-hackers |
On 7/3/20 10:11 AM, Peter Eisentraut wrote:
> On 2020-06-30 14:13, Michael Paquier wrote:
>> Attached is an updated patch, where I have tried to use a better
>> wording in all the code paths involved.
>
> This new patch doesn't work for me on MSYS2 yet.
>
> It fails right now in 010_pg_basebackup.pl at
>
> my $realTsDir = TestLib::perl2host("$shorter_tempdir/tblspc1");
>
> with chdir: No such file or directory. Because perl2host requires the
> parent directory of the argument to exist, but here it doesn't.
Yeah. I have a fix for that, which also checks to see if the grandparent
directory exists:
- chdir $parent or die "could not chdir \"$parent\": $!";
+ if (! chdir $parent)
+ {
+ $leaf = '/' . basename ($parent) . $leaf;
+ $parent = dirname $parent;
+ chdir $parent or die "could not chdir
\"$parent\": $!";
+ }
We could generalize it to walk all the way up the path, but I think this
is sufficient.
Incidentally, perl2host is arguably a bad name for this routine - there
is nothing perl-specific about the paths, they are provided by the msys
environment. Maybe virtual2host or some such would be a better name, or
even just host_path or native_path.
>
> If I add
>
> mkdir $shorter_tempdir;
>
> above it, then it proceeds past that point, but then the CREATE
> TABLESPACE command fails with No such file or directory. I think the
> call
>
> symlink "$tempdir", $shorter_tempdir;
>
> creates a directory inside $shorter_tempdir, since it now exists, per
> my above change, rather than in place of $shorter_tempdir.
>
> I think all of this is still a bit too fragile it needs further
> consideration.
The symlink built into msys2 perl is distinctly fragile. I was only able
to get it to work by doing this:
+ mkdir "$tempdir/tblspc1";
+ mkdir "$tempdir/tbl=spc2";
+ mkdir "$tempdir/$superlongname";
+ open (my $x, '>', "$tempdir/tblspc1/stuff") || die $!; print $x
"hi\n"; close($x);
+ open ($x, '>', "$tempdir/tbl=spc2/stuff") || die $!; print $x
"hi\n"; close($x);
+ open ($x, '>', "$tempdir/$superlongname/stuff") || die $!; print
$x "hi\n"; close($x);
symlink "$tempdir", $shorter_tempdir;
+ unlink "$tempdir/tblspc1/stuff";
+ unlink "$tempdir/tbl=spc2/stuff";
+ unlink "$tempdir/$superlongname/stuff";
which is sufficiently ugly that I don't think we should contemplate it.
Luckily there is an alternative, which doesn't require the use of
Win32::Symlink. Windows can be made to create junctions that function
exactly as expected quite easily - it's a builtin of the cmd.exe
processor, and it's been supported in at least every release since
Windows Vista. Here's a perl function that calls it:
sub dsymlink
{
my $oldname = shift;
my $newname = shift;
if ($windows_os)
{
$oldname = TestLib::perl2host($oldname);
$newname = TestLib::perl2host($newname);
$oldname =~ s,/,\\,g;
$newname =~ s,/,\\,g;
my $cmd = "cmd //c 'mklink /j $newname $oldname'";
system($cmd);
}
else
{
symlink $oldname, $newname;
}
die "No $newname" unless -e $newname;
}
It might need a little more quoting to be more robust.
Give that, we can simply replace
symlink "$tempdir", $shorter_tempdir;
with
dsymlink "$tempdir", $shorter_tempdir;
Then, with a little more sprinkling of perl2host the pg_basebackup tests
can be made to work on msys2.
I'm going to prepare patches along these lines.
cheers
andrew
--
Andrew Dunstan https://www.2ndQuadrant.com
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services
From | Date | Subject | |
---|---|---|---|
Next Message | Stephen Frost | 2020-07-08 14:00:37 | Re: Default setting for enable_hashagg_disk |
Previous Message | Rémi Lapeyre | 2020-07-08 13:51:28 | Re: [PATCH v2] Allow COPY "text" to output a header and add header matching mode to COPY FROM |