Re: TAP tests and symlinks on Windows

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

In response to

Responses

Browse pgsql-hackers by date

  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