TAP tests and symlinks on Windows

Started by Peter Eisentrautover 5 years ago34 messages
#1Peter Eisentraut
peter.eisentraut@2ndquadrant.com
1 attachment(s)

The tests

src/bin/pg_basebackup/t/010_pg_basebackup.pl
src/bin/pg_rewind/t/004_pg_xlog_symlink.pl

both contain a TAP skip notice "symlinks not supported on Windows".

This is untrue. Symlinks certainly work on Windows, and we have other
TAP tests using them, for example for tablespaces.

pg_rewind/t/004_pg_xlog_symlink.pl passes for me on Windows if I just
remove the skip stuff. My attached patch does that.

If I remove the skip stuff in pg_basebackup/t/010_pg_basebackup.pl, it
fails in various interesting ways. Apparently, some more work is needed
to get the various paths handled correct on Windows. (At least a
TestLib::perl2host call appears to be required.) I don't have the
enthusiasm to fix this right now, but my patch at least updates the
comment from "this isn't supported" to "this doesn't work correctly yet".

--
Peter Eisentraut http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services

Attachments:

0001-Enable-some-symlink-tests-on-Windows.patchtext/plain; charset=UTF-8; name=0001-Enable-some-symlink-tests-on-Windows.patch; x-mac-creator=0; x-mac-type=0Download
From 822583a01dc4f16d73390e266735d758e1507636 Mon Sep 17 00:00:00 2001
From: Peter Eisentraut <peter@eisentraut.org>
Date: Sat, 6 Jun 2020 09:38:54 +0200
Subject: [PATCH 1/2] Enable some symlink tests on Windows

---
 src/bin/pg_basebackup/t/010_pg_basebackup.pl |  6 +++---
 src/bin/pg_rewind/t/004_pg_xlog_symlink.pl   | 11 +----------
 2 files changed, 4 insertions(+), 13 deletions(-)

diff --git a/src/bin/pg_basebackup/t/010_pg_basebackup.pl b/src/bin/pg_basebackup/t/010_pg_basebackup.pl
index 208df557b8..a10d201378 100644
--- a/src/bin/pg_basebackup/t/010_pg_basebackup.pl
+++ b/src/bin/pg_basebackup/t/010_pg_basebackup.pl
@@ -211,11 +211,11 @@
 	'pg_basebackup tar with long name fails');
 unlink "$pgdata/$superlongname";
 
-# The following tests test symlinks. Windows doesn't have symlinks, so
-# skip on Windows.
+# The following tests need some work to work correctly on Windows, so
+# skip for now.
 SKIP:
 {
-	skip "symlinks not supported on Windows", 18 if ($windows_os);
+	skip "symlink tests not supported on Windows", 18 if ($windows_os);
 
 	# Move pg_replslot out of $pgdata and create a symlink to it.
 	$node->stop;
diff --git a/src/bin/pg_rewind/t/004_pg_xlog_symlink.pl b/src/bin/pg_rewind/t/004_pg_xlog_symlink.pl
index 639eeb9c91..e271a4ab16 100644
--- a/src/bin/pg_rewind/t/004_pg_xlog_symlink.pl
+++ b/src/bin/pg_rewind/t/004_pg_xlog_symlink.pl
@@ -6,16 +6,7 @@
 use File::Copy;
 use File::Path qw(rmtree);
 use TestLib;
-use Test::More;
-if ($windows_os)
-{
-	plan skip_all => 'symlinks not supported on Windows';
-	exit;
-}
-else
-{
-	plan tests => 5;
-}
+use Test::More tests => 5;
 
 use FindBin;
 use lib $FindBin::RealBin;
-- 
2.27.0

#2Michael Paquier
michael@paquier.xyz
In reply to: Peter Eisentraut (#1)
Re: TAP tests and symlinks on Windows

On Mon, Jun 08, 2020 at 02:44:31PM +0200, Peter Eisentraut wrote:

both contain a TAP skip notice "symlinks not supported on Windows".

This is untrue. Symlinks certainly work on Windows, and we have other TAP
tests using them, for example for tablespaces.

pg_rewind/t/004_pg_xlog_symlink.pl passes for me on Windows if I just remove
the skip stuff. My attached patch does that.

What's the version of your perl installation on Windows? With 5.22, I
am still seeing that symlink() is not implemented, causing the tests
of pg_rewind to blow in flight with your patch (MSVC 2015 here).
--
Michael

#3Peter Eisentraut
peter.eisentraut@2ndquadrant.com
In reply to: Michael Paquier (#2)
Re: TAP tests and symlinks on Windows

On 2020-06-09 09:19, Michael Paquier wrote:

On Mon, Jun 08, 2020 at 02:44:31PM +0200, Peter Eisentraut wrote:

both contain a TAP skip notice "symlinks not supported on Windows".

This is untrue. Symlinks certainly work on Windows, and we have other TAP
tests using them, for example for tablespaces.

pg_rewind/t/004_pg_xlog_symlink.pl passes for me on Windows if I just remove
the skip stuff. My attached patch does that.

What's the version of your perl installation on Windows? With 5.22, I
am still seeing that symlink() is not implemented, causing the tests
of pg_rewind to blow in flight with your patch (MSVC 2015 here).

I was using MSYS2 and the Perl version appears to have been 5.30.2.
Note sure which one of these two factors makes the difference.

--
Peter Eisentraut http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services

#4Juan José Santamaría Flecha
juanjo.santamaria@gmail.com
In reply to: Peter Eisentraut (#3)
Re: TAP tests and symlinks on Windows

On Tue, Jun 9, 2020 at 9:28 AM Peter Eisentraut <
peter.eisentraut@2ndquadrant.com> wrote:

On 2020-06-09 09:19, Michael Paquier wrote:

On Mon, Jun 08, 2020 at 02:44:31PM +0200, Peter Eisentraut wrote:

both contain a TAP skip notice "symlinks not supported on Windows".

This is untrue. Symlinks certainly work on Windows, and we have other

TAP

tests using them, for example for tablespaces.

pg_rewind/t/004_pg_xlog_symlink.pl passes for me on Windows if I just

remove

the skip stuff. My attached patch does that.

What's the version of your perl installation on Windows? With 5.22, I
am still seeing that symlink() is not implemented, causing the tests
of pg_rewind to blow in flight with your patch (MSVC 2015 here).

I was using MSYS2 and the Perl version appears to have been 5.30.2.
Note sure which one of these two factors makes the difference.

The difference seems to be MSYS2, it also fails for me if I do not
include 'Win32::Symlink' with Perl 5.30.2.

Regards,

Juan José Santamaría Flecha

#5Noname
ilmari@ilmari.org
In reply to: Juan José Santamaría Flecha (#4)
Re: TAP tests and symlinks on Windows

Juan José Santamaría Flecha <juanjo.santamaria@gmail.com> writes:

On Tue, Jun 9, 2020 at 9:28 AM Peter Eisentraut <
peter.eisentraut@2ndquadrant.com> wrote:

On 2020-06-09 09:19, Michael Paquier wrote:

On Mon, Jun 08, 2020 at 02:44:31PM +0200, Peter Eisentraut wrote:

both contain a TAP skip notice "symlinks not supported on Windows".

This is untrue. Symlinks certainly work on Windows, and we have other

TAP

tests using them, for example for tablespaces.

pg_rewind/t/004_pg_xlog_symlink.pl passes for me on Windows if I just

remove

the skip stuff. My attached patch does that.

What's the version of your perl installation on Windows? With 5.22, I
am still seeing that symlink() is not implemented, causing the tests
of pg_rewind to blow in flight with your patch (MSVC 2015 here).

I was using MSYS2 and the Perl version appears to have been 5.30.2.
Note sure which one of these two factors makes the difference.

The difference seems to be MSYS2, it also fails for me if I do not
include 'Win32::Symlink' with Perl 5.30.2.

Amusingly, Win32::Symlink uses a copy of our pgsymlink(), which emulates
symlinks via junction points:

https://metacpan.org/source/AUDREYT/Win32-Symlink-0.06/pgsymlink.c

A portable way of using symlinks if possible would be:

# In a BEGIN block because it overrides CORE::GLOBAL::symlink, which
# only takes effect on code that's compiled after the override is
# installed. We don't care if it fails, since it works without on
# some Windows perls.
BEGIN {
eval { require Win32::Symlink; Win32::Symlink->import; }
}

# symlink() throws an exception if t
if (not eval { symlink("",""); 1; })
{
plan skip_all => 'symlinks not supported';
}
else
{
plan tests => 5;
}

Plus a note in the Win32 docs that Win32::Symlink may be required to run
some tests on some Perl/Windows versions..

- ilmari
--
- Twitter seems more influential [than blogs] in the 'gets reported in
the mainstream press' sense at least. - Matt McLeod
- That'd be because the content of a tweet is easier to condense down
to a mainstream media article. - Calle Dybedahl

#6Michael Paquier
michael@paquier.xyz
In reply to: Noname (#5)
Re: TAP tests and symlinks on Windows

On Tue, Jun 09, 2020 at 11:26:19AM +0100, Dagfinn Ilmari Mannsåker wrote:

Amusingly, Win32::Symlink uses a copy of our pgsymlink(), which emulates
symlinks via junction points:

https://metacpan.org/source/AUDREYT/Win32-Symlink-0.06/pgsymlink.c

Oh, interesting point. Thanks for the reference!

A portable way of using symlinks if possible would be:

# In a BEGIN block because it overrides CORE::GLOBAL::symlink, which
# only takes effect on code that's compiled after the override is
# installed. We don't care if it fails, since it works without on
# some Windows perls.
[...]

Plus a note in the Win32 docs that Win32::Symlink may be required to run
some tests on some Perl/Windows versions..

Planting such a check in individual scripts is not a good idea because
it would get forgotten. The best way to handle that is to add a new
check in the BEGIN block of TestLib.pm. Note that we already do that
with createFile, OsFHandleOpen and CloseHandle. Now the question is:
do we really want to make this a hard requirement? I would like to
answer yes so as we make sure that this gets always tested, and this
needs proper documentation as you say. Now it would be also possible
to check if the API is present in the BEGIN block of TestLib.pm, and
then use an independent variable similar to what we do with
$use_unix_sockets to decide if tests should be skipped or not, but you
cannot know if this gets actually, or ever, tested.
--
Michael

#7Juan José Santamaría Flecha
juanjo.santamaria@gmail.com
In reply to: Michael Paquier (#6)
Re: TAP tests and symlinks on Windows

On Fri, Jun 12, 2020 at 9:00 AM Michael Paquier <michael@paquier.xyz> wrote:

On Tue, Jun 09, 2020 at 11:26:19AM +0100, Dagfinn Ilmari Mannsåker wrote:

Plus a note in the Win32 docs that Win32::Symlink may be required to run
some tests on some Perl/Windows versions..

Planting such a check in individual scripts is not a good idea because
it would get forgotten. The best way to handle that is to add a new
check in the BEGIN block of TestLib.pm. Note that we already do that
with createFile, OsFHandleOpen and CloseHandle. Now the question is:
do we really want to make this a hard requirement? I would like to
answer yes so as we make sure that this gets always tested, and this
needs proper documentation as you say. Now it would be also possible
to check if the API is present in the BEGIN block of TestLib.pm, and
then use an independent variable similar to what we do with
$use_unix_sockets to decide if tests should be skipped or not, but you
cannot know if this gets actually, or ever, tested.

The first thing that comes to mind is adding an option to vcregress to
choose whether symlinks will be tested or skipped, would that be an
acceptable solution?

Regards,

Juan José Santamaría Flecha

Show quoted text
#8Michael Paquier
michael@paquier.xyz
In reply to: Juan José Santamaría Flecha (#7)
Re: TAP tests and symlinks on Windows

On Fri, Jun 12, 2020 at 02:02:52PM +0200, Juan José Santamaría Flecha wrote:

The first thing that comes to mind is adding an option to vcregress to
choose whether symlinks will be tested or skipped, would that be an
acceptable solution?

My take would be to actually enforce that as a requirement for 14~ if
that works reliably, and of course not backpatch that change as that's
clearly an improvement and not a bug fix. It would be good to check
the status of each buildfarm member first though. And I would need to
also check my own stuff to begin with..
--
Michael

#9Michael Paquier
michael@paquier.xyz
In reply to: Michael Paquier (#8)
1 attachment(s)
Re: TAP tests and symlinks on Windows

On Sat, Jun 13, 2020 at 03:00:54PM +0900, Michael Paquier wrote:

My take would be to actually enforce that as a requirement for 14~ if
that works reliably, and of course not backpatch that change as that's
clearly an improvement and not a bug fix. It would be good to check
the status of each buildfarm member first though. And I would need to
also check my own stuff to begin with..

So, I have been looking at that. And indeed as Peter said we are
visibly missing one call to perl2host in 010_pg_basebackup.pl.

Another thing I spotted is that Win32::Symlink does not allow to
detect properly if a path is a symlink using -l, causing one of the
tests of pg_basebackup to fail when checking if a tablespace path has
been updted. It would be good to get more people to test this patch
with different environments than mine. I am also adding Andrew
Dunstan in CC as the owner of the buildfarm animals running currently
TAP tests for confirmation about the presence of Win32::Symlink
there as I am afraid it would cause failures: drongo, fairywen,
jacana and bowerbird.
--
Michael

Attachments:

win32-readlink-tap.patchtext/x-diff; charset=us-asciiDownload
diff --git a/src/bin/pg_basebackup/t/010_pg_basebackup.pl b/src/bin/pg_basebackup/t/010_pg_basebackup.pl
index 208df557b8..17643489d1 100644
--- a/src/bin/pg_basebackup/t/010_pg_basebackup.pl
+++ b/src/bin/pg_basebackup/t/010_pg_basebackup.pl
@@ -17,7 +17,7 @@ my $tempdir = TestLib::tempdir;
 my $node = get_new_node('main');
 
 # Set umask so test directories and files are created with default permissions
-umask(0077);
+umask(0077) if (!$windows_os);
 
 # Initialize node without replication settings
 $node->init(extra => ['--data-checksums']);
@@ -211,154 +211,168 @@ $node->command_fails(
 	'pg_basebackup tar with long name fails');
 unlink "$pgdata/$superlongname";
 
-# The following tests test symlinks. Windows doesn't have symlinks, so
-# skip on Windows.
-SKIP:
+# Move pg_replslot out of $pgdata and create a symlink to it.
+$node->stop;
+
+# Set umask so test directories and files are created with group permissions
+if (!$windows_os)
 {
-	skip "symlinks not supported on Windows", 18 if ($windows_os);
-
-	# Move pg_replslot out of $pgdata and create a symlink to it.
-	$node->stop;
-
-	# Set umask so test directories and files are created with group permissions
-	umask(0027);
+	umask(0027) if (!$windows_os);
 
 	# Enable group permissions on PGDATA
 	chmod_recursive("$pgdata", 0750, 0640);
+}
 
-	rename("$pgdata/pg_replslot", "$tempdir/pg_replslot")
-	  or BAIL_OUT "could not move $pgdata/pg_replslot";
-	symlink("$tempdir/pg_replslot", "$pgdata/pg_replslot")
-	  or BAIL_OUT "could not symlink to $pgdata/pg_replslot";
+rename("$pgdata/pg_replslot", "$tempdir/pg_replslot")
+  or BAIL_OUT "could not move $pgdata/pg_replslot";
+symlink("$tempdir/pg_replslot", "$pgdata/pg_replslot")
+  or BAIL_OUT "could not symlink to $pgdata/pg_replslot";
 
-	$node->start;
+$node->start;
 
-	# Create a temporary directory in the system location and symlink it
-	# to our physical temp location.  That way we can use shorter names
-	# for the tablespace directories, which hopefully won't run afoul of
-	# the 99 character length limit.
-	my $shorter_tempdir = TestLib::tempdir_short . "/tempdir";
-	symlink "$tempdir", $shorter_tempdir;
+# Create a temporary directory in the system location and symlink it
+# to our physical temp location.  That way we can use shorter names
+# for the tablespace directories, which hopefully won't run afoul of
+# the 99 character length limit.
+my $shorter_tempdir = TestLib::tempdir_short . "/tempdir";
+my $realTsDir       = TestLib::perl2host("$shorter_tempdir/tblspc1");
+symlink "$tempdir", $shorter_tempdir;
 
-	mkdir "$tempdir/tblspc1";
-	$node->safe_psql('postgres',
-		"CREATE TABLESPACE tblspc1 LOCATION '$shorter_tempdir/tblspc1';");
-	$node->safe_psql('postgres',
-		"CREATE TABLE test1 (a int) TABLESPACE tblspc1;");
-	$node->command_ok([ 'pg_basebackup', '-D', "$tempdir/tarbackup2", '-Ft' ],
-		'tar format with tablespaces');
-	ok(-f "$tempdir/tarbackup2/base.tar", 'backup tar was created');
-	my @tblspc_tars = glob "$tempdir/tarbackup2/[0-9]*.tar";
-	is(scalar(@tblspc_tars), 1, 'one tablespace tar was created');
-	rmtree("$tempdir/tarbackup2");
+mkdir "$tempdir/tblspc1";
+$node->safe_psql('postgres',
+	"CREATE TABLESPACE tblspc1 LOCATION '$realTsDir';");
+$node->safe_psql('postgres',
+	"CREATE TABLE test1 (a int) TABLESPACE tblspc1;");
+$node->command_ok([ 'pg_basebackup', '-D', "$tempdir/tarbackup2", '-Ft' ],
+	'tar format with tablespaces');
+ok(-f "$tempdir/tarbackup2/base.tar", 'backup tar was created');
+my @tblspc_tars = glob "$tempdir/tarbackup2/[0-9]*.tar";
+is(scalar(@tblspc_tars), 1, 'one tablespace tar was created');
+rmtree("$tempdir/tarbackup2");
 
-	# Create an unlogged table to test that forks other than init are not copied.
-	$node->safe_psql('postgres',
-		'CREATE UNLOGGED TABLE tblspc1_unlogged (id int) TABLESPACE tblspc1;'
-	);
+# Create an unlogged table to test that forks other than init are not copied.
+$node->safe_psql('postgres',
+	'CREATE UNLOGGED TABLE tblspc1_unlogged (id int) TABLESPACE tblspc1;');
 
-	my $tblspc1UnloggedPath = $node->safe_psql('postgres',
-		q{select pg_relation_filepath('tblspc1_unlogged')});
+my $tblspc1UnloggedPath = $node->safe_psql('postgres',
+	q{select pg_relation_filepath('tblspc1_unlogged')});
 
-	# Make sure main and init forks exist
-	ok( -f "$pgdata/${tblspc1UnloggedPath}_init",
-		'unlogged init fork in tablespace');
-	ok(-f "$pgdata/$tblspc1UnloggedPath", 'unlogged main fork in tablespace');
+# Make sure main and init forks exist
+ok( -f "$pgdata/${tblspc1UnloggedPath}_init",
+	'unlogged init fork in tablespace');
+ok(-f "$pgdata/$tblspc1UnloggedPath", 'unlogged main fork in tablespace');
 
-	# Create files that look like temporary relations to ensure they are ignored
-	# in a tablespace.
-	my @tempRelationFiles = qw(t888_888 t888888_888888_vm.1);
-	my $tblSpc1Id         = basename(
+# Create files that look like temporary relations to ensure they are ignored
+# in a tablespace.
+@tempRelationFiles = qw(t888_888 t888888_888888_vm.1);
+my $tblSpc1Id = basename(
+	dirname(
 		dirname(
-			dirname(
-				$node->safe_psql(
-					'postgres', q{select pg_relation_filepath('test1')}))));
+			$node->safe_psql(
+				'postgres', q{select pg_relation_filepath('test1')}))));
 
-	foreach my $filename (@tempRelationFiles)
-	{
-		append_to_file(
-			"$shorter_tempdir/tblspc1/$tblSpc1Id/$postgresOid/$filename",
-			'TEMP_RELATION');
-	}
+foreach my $filename (@tempRelationFiles)
+{
+	append_to_file(
+		"$shorter_tempdir/tblspc1/$tblSpc1Id/$postgresOid/$filename",
+		'TEMP_RELATION');
+}
 
-	$node->command_fails(
-		[ 'pg_basebackup', '-D', "$tempdir/backup1", '-Fp' ],
-		'plain format with tablespaces fails without tablespace mapping');
+$node->command_fails(
+	[ 'pg_basebackup', '-D', "$tempdir/backup1", '-Fp' ],
+	'plain format with tablespaces fails without tablespace mapping');
+
+$node->command_ok(
+	[
+		'pg_basebackup', '-D', "$tempdir/backup1", '-Fp',
+		"-T$shorter_tempdir/tblspc1=$tempdir/tbackup/tblspc1"
+	],
+	'plain format with tablespaces succeeds with tablespace mapping');
+ok(-d "$tempdir/tbackup/tblspc1", 'tablespace was relocated');
+
+# Symlink checks are not supported on Windows.  Win32::Symlink works
+# around this situation by using junction points (actually PostgreSQL
+# approach on the problem), and -l is not able to detect that situation.
+SKIP:
+{
+	skip "symlink check not implemented on Windows", 1
+	  if ($windows_os);
 
-	$node->command_ok(
-		[
-			'pg_basebackup', '-D', "$tempdir/backup1", '-Fp',
-			"-T$shorter_tempdir/tblspc1=$tempdir/tbackup/tblspc1"
-		],
-		'plain format with tablespaces succeeds with tablespace mapping');
-	ok(-d "$tempdir/tbackup/tblspc1", 'tablespace was relocated');
 	opendir(my $dh, "$pgdata/pg_tblspc") or die;
-	ok( (   grep {
-				-l "$tempdir/backup1/pg_tblspc/$_"
-				  and readlink "$tempdir/backup1/pg_tblspc/$_" eq
-				  "$tempdir/tbackup/tblspc1"
-			} readdir($dh)),
-		"tablespace symlink was updated");
+	while (readdir($dh))
+	{
+		my $tbspace_backup_dir = "$tempdir/backup1/pg_tblspc/$_";
+		if (-l $tbspace_backup_dir)
+		{
+			my $tb_link = readlink($tbspace_backup_dir);
+			ok( $tb_link eq "$tempdir/tbackup/tblspc1",
+				"tablespace symlink was updated");
+		}
+	}
 	closedir $dh;
+}
+
+# Group access should be enabled on all backup files
+SKIP:
+{
+	skip "unix-style permissions not supported on Windows", 1
+	  if ($windows_os);
 
-	# Group access should be enabled on all backup files
 	ok(check_mode_recursive("$tempdir/backup1", 0750, 0640),
 		"check backup dir permissions");
-
-	# Unlogged relation forks other than init should not be copied
-	my ($tblspc1UnloggedBackupPath) =
-	  $tblspc1UnloggedPath =~ /[^\/]*\/[^\/]*\/[^\/]*$/g;
-
-	ok(-f "$tempdir/tbackup/tblspc1/${tblspc1UnloggedBackupPath}_init",
-		'unlogged init fork in tablespace backup');
-	ok(!-f "$tempdir/tbackup/tblspc1/$tblspc1UnloggedBackupPath",
-		'unlogged main fork not in tablespace backup');
-
-	# Temp relations should not be copied.
-	foreach my $filename (@tempRelationFiles)
-	{
-		ok( !-f "$tempdir/tbackup/tblspc1/$tblSpc1Id/$postgresOid/$filename",
-			"[tblspc1]/$postgresOid/$filename not copied");
-
-		# Also remove temp relation files or tablespace drop will fail.
-		my $filepath =
-		  "$shorter_tempdir/tblspc1/$tblSpc1Id/$postgresOid/$filename";
-
-		unlink($filepath)
-		  or BAIL_OUT("unable to unlink $filepath");
-	}
-
-	ok( -d "$tempdir/backup1/pg_replslot",
-		'pg_replslot symlink copied as directory');
-	rmtree("$tempdir/backup1");
-
-	mkdir "$tempdir/tbl=spc2";
-	$node->safe_psql('postgres', "DROP TABLE test1;");
-	$node->safe_psql('postgres', "DROP TABLE tblspc1_unlogged;");
-	$node->safe_psql('postgres', "DROP TABLESPACE tblspc1;");
-	$node->safe_psql('postgres',
-		"CREATE TABLESPACE tblspc2 LOCATION '$shorter_tempdir/tbl=spc2';");
-	$node->command_ok(
-		[
-			'pg_basebackup', '-D', "$tempdir/backup3", '-Fp',
-			"-T$shorter_tempdir/tbl\\=spc2=$tempdir/tbackup/tbl\\=spc2"
-		],
-		'mapping tablespace with = sign in path');
-	ok(-d "$tempdir/tbackup/tbl=spc2",
-		'tablespace with = sign was relocated');
-	$node->safe_psql('postgres', "DROP TABLESPACE tblspc2;");
-	rmtree("$tempdir/backup3");
-
-	mkdir "$tempdir/$superlongname";
-	$node->safe_psql('postgres',
-		"CREATE TABLESPACE tblspc3 LOCATION '$tempdir/$superlongname';");
-	$node->command_ok(
-		[ 'pg_basebackup', '-D', "$tempdir/tarbackup_l3", '-Ft' ],
-		'pg_basebackup tar with long symlink target');
-	$node->safe_psql('postgres', "DROP TABLESPACE tblspc3;");
-	rmtree("$tempdir/tarbackup_l3");
 }
 
+# Unlogged relation forks other than init should not be copied
+my ($tblspc1UnloggedBackupPath) =
+  $tblspc1UnloggedPath =~ /[^\/]*\/[^\/]*\/[^\/]*$/g;
+
+ok(-f "$tempdir/tbackup/tblspc1/${tblspc1UnloggedBackupPath}_init",
+	'unlogged init fork in tablespace backup');
+ok(!-f "$tempdir/tbackup/tblspc1/$tblspc1UnloggedBackupPath",
+	'unlogged main fork not in tablespace backup');
+
+# Temp relations should not be copied.
+foreach my $filename (@tempRelationFiles)
+{
+	ok(!-f "$tempdir/tbackup/tblspc1/$tblSpc1Id/$postgresOid/$filename",
+		"[tblspc1]/$postgresOid/$filename not copied");
+
+	# Also remove temp relation files or tablespace drop will fail.
+	my $filepath =
+	  "$shorter_tempdir/tblspc1/$tblSpc1Id/$postgresOid/$filename";
+
+	unlink($filepath)
+	  or BAIL_OUT("unable to unlink $filepath");
+}
+
+ok( -d "$tempdir/backup1/pg_replslot",
+	'pg_replslot symlink copied as directory');
+rmtree("$tempdir/backup1");
+
+mkdir "$tempdir/tbl=spc2";
+$node->safe_psql('postgres', "DROP TABLE test1;");
+$node->safe_psql('postgres', "DROP TABLE tblspc1_unlogged;");
+$node->safe_psql('postgres', "DROP TABLESPACE tblspc1;");
+$node->safe_psql('postgres',
+	"CREATE TABLESPACE tblspc2 LOCATION '$shorter_tempdir/tbl=spc2';");
+$node->command_ok(
+	[
+		'pg_basebackup', '-D', "$tempdir/backup3", '-Fp',
+		"-T$shorter_tempdir/tbl\\=spc2=$tempdir/tbackup/tbl\\=spc2"
+	],
+	'mapping tablespace with = sign in path');
+ok(-d "$tempdir/tbackup/tbl=spc2", 'tablespace with = sign was relocated');
+$node->safe_psql('postgres', "DROP TABLESPACE tblspc2;");
+rmtree("$tempdir/backup3");
+
+mkdir "$tempdir/$superlongname";
+$node->safe_psql('postgres',
+	"CREATE TABLESPACE tblspc3 LOCATION '$tempdir/$superlongname';");
+$node->command_ok([ 'pg_basebackup', '-D', "$tempdir/tarbackup_l3", '-Ft' ],
+	'pg_basebackup tar with long symlink target');
+$node->safe_psql('postgres', "DROP TABLESPACE tblspc3;");
+rmtree("$tempdir/tarbackup_l3");
+
 $node->command_ok([ 'pg_basebackup', '-D', "$tempdir/backupR", '-R' ],
 	'pg_basebackup -R runs');
 ok(-f "$tempdir/backupR/postgresql.auto.conf", 'postgresql.auto.conf exists');
diff --git a/src/bin/pg_rewind/t/004_pg_xlog_symlink.pl b/src/bin/pg_rewind/t/004_pg_xlog_symlink.pl
index 639eeb9c91..e271a4ab16 100644
--- a/src/bin/pg_rewind/t/004_pg_xlog_symlink.pl
+++ b/src/bin/pg_rewind/t/004_pg_xlog_symlink.pl
@@ -6,16 +6,7 @@ use warnings;
 use File::Copy;
 use File::Path qw(rmtree);
 use TestLib;
-use Test::More;
-if ($windows_os)
-{
-	plan skip_all => 'symlinks not supported on Windows';
-	exit;
-}
-else
-{
-	plan tests => 5;
-}
+use Test::More tests => 5;
 
 use FindBin;
 use lib $FindBin::RealBin;
diff --git a/src/test/perl/TestLib.pm b/src/test/perl/TestLib.pm
index d579d5c177..612f3b111e 100644
--- a/src/test/perl/TestLib.pm
+++ b/src/test/perl/TestLib.pm
@@ -118,6 +118,9 @@ BEGIN
 	{
 		require Win32API::File;
 		Win32API::File->import(qw(createFile OsFHandleOpen CloseHandle));
+
+		require Win32::Symlink;
+		Win32::Symlink->import(qw(readlink symlink));
 	}
 
 	# Specifies whether to use Unix sockets for test setups.  On
#10Juan José Santamaría Flecha
juanjo.santamaria@gmail.com
In reply to: Michael Paquier (#9)
Re: TAP tests and symlinks on Windows

On Mon, Jun 15, 2020 at 8:23 AM Michael Paquier <michael@paquier.xyz> wrote:

Another thing I spotted is that Win32::Symlink does not allow to
detect properly if a path is a symlink using -l, causing one of the
tests of pg_basebackup to fail when checking if a tablespace path has
been updted. It would be good to get more people to test this patch
with different environments than mine. I am also adding Andrew
Dunstan in CC as the owner of the buildfarm animals running currently
TAP tests for confirmation about the presence of Win32::Symlink
there as I am afraid it would cause failures: drongo, fairywen,
jacana and bowerbird.

This patch works on my Windows 10 / Visual Studio 2019 / Perl 5.30.2
machine.

Regards,

Juan José Santamaría Flecha

#11Andrew Dunstan
andrew.dunstan@2ndquadrant.com
In reply to: Michael Paquier (#9)
Re: TAP tests and symlinks on Windows

On 6/15/20 2:23 AM, Michael Paquier wrote:

On Sat, Jun 13, 2020 at 03:00:54PM +0900, Michael Paquier wrote:

My take would be to actually enforce that as a requirement for 14~ if
that works reliably, and of course not backpatch that change as that's
clearly an improvement and not a bug fix. It would be good to check
the status of each buildfarm member first though. And I would need to
also check my own stuff to begin with..

So, I have been looking at that. And indeed as Peter said we are
visibly missing one call to perl2host in 010_pg_basebackup.pl.

Another thing I spotted is that Win32::Symlink does not allow to
detect properly if a path is a symlink using -l, causing one of the
tests of pg_basebackup to fail when checking if a tablespace path has
been updted. It would be good to get more people to test this patch
with different environments than mine. I am also adding Andrew
Dunstan in CC as the owner of the buildfarm animals running currently
TAP tests for confirmation about the presence of Win32::Symlink
there as I am afraid it would cause failures: drongo, fairywen,
jacana and bowerbird.

Not one of them has it.

I think we'll need a dynamic test for its presence rather than just
assuming it's there. (Use require in an eval for this).

However, since all of them would currently fail we wouldn't actually
have any test coverage. I could see about installing it on one or two
animals (jacana would be a problem, it's using a very old and limited
perl to run TAP tests.)

cheers

andrew

--
Andrew Dunstan https://www.2ndQuadrant.com
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services

#12Peter Eisentraut
peter.eisentraut@2ndquadrant.com
In reply to: Juan José Santamaría Flecha (#4)
Re: TAP tests and symlinks on Windows

On 2020-06-09 09:33, Juan José Santamaría Flecha wrote:

The difference seems to be MSYS2, it also fails for me if I do not
include 'Win32::Symlink' with Perl 5.30.2.

MSYS2, which is basically Cygwin, emulates symlinks with junction
points, so this happens to work for our purpose. We could therefore
enable these tests in that environment, if we could come up with a
reliable way to detect it.

Also, if we are going to add Win32::Symlink to the mix, we should make
sure things continue to work correctly under MSYS2.

--
Peter Eisentraut http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services

#13Andrew Dunstan
andrew.dunstan@2ndquadrant.com
In reply to: Peter Eisentraut (#12)
Re: TAP tests and symlinks on Windows

On 6/16/20 8:24 AM, Peter Eisentraut wrote:

On 2020-06-09 09:33, Juan José Santamaría Flecha wrote:

The difference seems to be MSYS2, it also fails for me if I do not
include 'Win32::Symlink' with Perl 5.30.2.

MSYS2, which is basically Cygwin, emulates symlinks with junction
points, so this happens to work for our purpose.  We could therefore
enable these tests in that environment, if we could come up with a
reliable way to detect it.

From src/bin/pg_dump/t/010_dump_connstr.pl:

if ($^O eq 'msys' && `uname -or` =~ /^[2-9].*Msys/)

cheers

andrew

--
Andrew Dunstan https://www.2ndQuadrant.com
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services

#14Michael Paquier
michael@paquier.xyz
In reply to: Andrew Dunstan (#13)
Re: TAP tests and symlinks on Windows

On Tue, Jun 16, 2020 at 08:32:03AM -0400, Andrew Dunstan wrote:

On 6/16/20 8:24 AM, Peter Eisentraut wrote:

MSYS2, which is basically Cygwin, emulates symlinks with junction
points, so this happens to work for our purpose.  We could therefore
enable these tests in that environment, if we could come up with a
reliable way to detect it.

Hmm. In this case does perl's -l think that a junction point is
corrently a soft link or not? We have a check based on that in
pg_basebackup's test and -l fails when it sees to a junction point,
forcing us to skip this test.

From src/bin/pg_dump/t/010_dump_connstr.pl:

if ($^O eq 'msys' && `uname -or` =~ /^[2-9].*Msys/)

Smart. This could become a central variable in TestLib.pm.
--
Michael

#15Michael Paquier
michael@paquier.xyz
In reply to: Andrew Dunstan (#11)
Re: TAP tests and symlinks on Windows

On Tue, Jun 16, 2020 at 07:53:26AM -0400, Andrew Dunstan wrote:

Not one of them has it.

Argh.

I think we'll need a dynamic test for its presence rather than just
assuming it's there. (Use require in an eval for this).

Sure. No problem with implementing an automatic detection.

However, since all of them would currently fail we wouldn't actually
have any test coverage. I could see about installing it on one or two
animals (jacana would be a problem, it's using a very old and limited
perl to run TAP tests.)

Okay. This could be a problem as jacana is proving to have good
coverage AFAIK. So it looks like we are really heading in the
direction is still skipping the test if there is no support for
symlink in the environment. At least that makes less diffs in the
patch.
--
Michael

#16Michael Paquier
michael@paquier.xyz
In reply to: Michael Paquier (#15)
1 attachment(s)
Re: TAP tests and symlinks on Windows

On Wed, Jun 17, 2020 at 04:44:34PM +0900, Michael Paquier wrote:

Okay. This could be a problem as jacana is proving to have good
coverage AFAIK. So it looks like we are really heading in the
direction is still skipping the test if there is no support for
symlink in the environment. At least that makes less diffs in the
patch.

I have implemented a patch based on the feedback received that does
the following, tested with all three patterns (MSVC only on Windows):
- Assume that all non-Windows platform have a proper symlink
implementation for perl.
- If on Windows, check for the presence of Win32::Symlink:
-- If the module is not detected, skip the tests not supported.
-- If the module is detected, run them.

I have added this patch to the next commit fest:
https://commitfest.postgresql.org/28/2612/

Thanks,
--
Michael

Attachments:

win32-readlink-tap-v2.patchtext/x-diff; charset=us-asciiDownload
diff --git a/src/bin/pg_basebackup/t/010_pg_basebackup.pl b/src/bin/pg_basebackup/t/010_pg_basebackup.pl
index 208df557b8..715bf9a309 100644
--- a/src/bin/pg_basebackup/t/010_pg_basebackup.pl
+++ b/src/bin/pg_basebackup/t/010_pg_basebackup.pl
@@ -211,11 +211,11 @@ $node->command_fails(
 	'pg_basebackup tar with long name fails');
 unlink "$pgdata/$superlongname";
 
-# The following tests test symlinks. Windows doesn't have symlinks, so
-# skip on Windows.
+# The following tests test symlinks. Windows may not have symlinks, so
+# skip there.
 SKIP:
 {
-	skip "symlinks not supported on Windows", 18 if ($windows_os);
+	skip "symlinks not supported on this platform", 18 if (!$symlink_support);
 
 	# Move pg_replslot out of $pgdata and create a symlink to it.
 	$node->stop;
@@ -238,11 +238,12 @@ SKIP:
 	# for the tablespace directories, which hopefully won't run afoul of
 	# the 99 character length limit.
 	my $shorter_tempdir = TestLib::tempdir_short . "/tempdir";
+	my $realTsDir       = TestLib::perl2host("$shorter_tempdir/tblspc1");
 	symlink "$tempdir", $shorter_tempdir;
 
 	mkdir "$tempdir/tblspc1";
 	$node->safe_psql('postgres',
-		"CREATE TABLESPACE tblspc1 LOCATION '$shorter_tempdir/tblspc1';");
+		"CREATE TABLESPACE tblspc1 LOCATION '$realTsDir';");
 	$node->safe_psql('postgres',
 		"CREATE TABLE test1 (a int) TABLESPACE tblspc1;");
 	$node->command_ok([ 'pg_basebackup', '-D', "$tempdir/tarbackup2", '-Ft' ],
@@ -292,18 +293,33 @@ SKIP:
 		],
 		'plain format with tablespaces succeeds with tablespace mapping');
 	ok(-d "$tempdir/tbackup/tblspc1", 'tablespace was relocated');
-	opendir(my $dh, "$pgdata/pg_tblspc") or die;
-	ok( (   grep {
-				-l "$tempdir/backup1/pg_tblspc/$_"
-				  and readlink "$tempdir/backup1/pg_tblspc/$_" eq
-				  "$tempdir/tbackup/tblspc1"
-			} readdir($dh)),
-		"tablespace symlink was updated");
-	closedir $dh;
+
+	# Symlink checks are not supported on Windows.  Win32::Symlink works
+	# around this situation by using junction points (actually PostgreSQL
+	# approach on the problem), and -l is not able to detect that situation.
+  SKIP:
+	{
+		skip "symlink check not implemented on Windows", 1
+		  if ($windows_os);
+		opendir(my $dh, "$pgdata/pg_tblspc") or die;
+		ok( (   grep {
+					-l "$tempdir/backup1/pg_tblspc/$_"
+					  and readlink "$tempdir/backup1/pg_tblspc/$_" eq
+					  "$tempdir/tbackup/tblspc1"
+				} readdir($dh)),
+			"tablespace symlink was updated");
+		closedir $dh;
+	}
 
 	# Group access should be enabled on all backup files
-	ok(check_mode_recursive("$tempdir/backup1", 0750, 0640),
-		"check backup dir permissions");
+  SKIP:
+	{
+		skip "unix-style permissions not supported on Windows", 1
+		  if ($windows_os);
+
+		ok(check_mode_recursive("$tempdir/backup1", 0750, 0640),
+			"check backup dir permissions");
+	}
 
 	# Unlogged relation forks other than init should not be copied
 	my ($tblspc1UnloggedBackupPath) =
diff --git a/src/bin/pg_rewind/t/004_pg_xlog_symlink.pl b/src/bin/pg_rewind/t/004_pg_xlog_symlink.pl
index 639eeb9c91..0797142588 100644
--- a/src/bin/pg_rewind/t/004_pg_xlog_symlink.pl
+++ b/src/bin/pg_rewind/t/004_pg_xlog_symlink.pl
@@ -7,9 +7,9 @@ use File::Copy;
 use File::Path qw(rmtree);
 use TestLib;
 use Test::More;
-if ($windows_os)
+if (!$symlink_support)
 {
-	plan skip_all => 'symlinks not supported on Windows';
+	plan skip_all => 'symlinks not supported on this platform';
 	exit;
 }
 else
diff --git a/src/test/perl/TestLib.pm b/src/test/perl/TestLib.pm
index d579d5c177..e9cd045f06 100644
--- a/src/test/perl/TestLib.pm
+++ b/src/test/perl/TestLib.pm
@@ -22,6 +22,7 @@ TestLib - helper module for writing PostgreSQL's C<prove> tests.
 
   # Miscellanea
   print "on Windows" if $TestLib::windows_os;
+  print "supports symlinks" if $TestLib::symlink_support;
   my $path = TestLib::perl2host($backup_dir);
   ok(check_mode_recursive($stream_dir, 0700, 0600),
     "check stream dir permissions");
@@ -84,10 +85,12 @@ our @EXPORT = qw(
   command_checks_all
 
   $windows_os
+  $symlink_support
   $use_unix_sockets
 );
 
-our ($windows_os, $use_unix_sockets, $tmp_check, $log_path, $test_logfile);
+our ($windows_os, $symlink_support, $use_unix_sockets, $tmp_check, $log_path,
+	$test_logfile);
 
 BEGIN
 {
@@ -120,6 +123,28 @@ BEGIN
 		Win32API::File->import(qw(createFile OsFHandleOpen CloseHandle));
 	}
 
+	# Check if this environment has support for symlinks.  Windows
+	# may provide an equivalent implementation based on junction
+	# points thanks to Win32::Symlink.  Non-Windows platforms should
+	# have an implementation natively available.
+	if ($windows_os)
+	{
+		eval { require Win32::Symlink; };
+		if ($@)
+		{
+			$symlink_support = 0;
+		}
+		else
+		{
+			$symlink_support = 1;
+			Win32::Symlink->import(qw(readlink symlink));
+		}
+	}
+	else
+	{
+		$symlink_support = 1;
+	}
+
 	# Specifies whether to use Unix sockets for test setups.  On
 	# Windows we don't use them by default since it's not universally
 	# supported, but it can be overridden if desired.
@@ -137,6 +162,10 @@ BEGIN
 
 Set to true when running under Windows, except on Cygwin.
 
+=item C<$symlink_support>
+
+Set to true when running on a platform that supports symlinks.
+
 =back
 
 =cut
#17Peter Eisentraut
peter.eisentraut@2ndquadrant.com
In reply to: Michael Paquier (#16)
Re: TAP tests and symlinks on Windows

On 2020-06-23 12:55, Michael Paquier wrote:

I have implemented a patch based on the feedback received that does
the following, tested with all three patterns (MSVC only on Windows):
- Assume that all non-Windows platform have a proper symlink
implementation for perl.
- If on Windows, check for the presence of Win32::Symlink:
-- If the module is not detected, skip the tests not supported.
-- If the module is detected, run them.

We should be more accurate about things like this:

+# The following tests test symlinks. Windows may not have symlinks, so
+# skip there.

The issue isn't whether Windows has symlinks, since all versions of
Windows supported by PostgreSQL do (AFAIK). The issue is only whether
the Perl installation that runs the tests has symlink support. And that
is only necessary if the test itself wants to create or inspect
symlinks. For example, there are existing tests involving tablespaces
that work just fine on Windows.

Relatedly, your patch ends up skipping the tests on MSYS2, even though
Perl supports symlinks there out of the box.

--
Peter Eisentraut http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services

#18Michael Paquier
michael@paquier.xyz
In reply to: Peter Eisentraut (#17)
Re: TAP tests and symlinks on Windows

On Fri, Jun 26, 2020 at 02:00:37PM +0200, Peter Eisentraut wrote:

We should be more accurate about things like this:

+# The following tests test symlinks. Windows may not have symlinks, so
+# skip there.

The issue isn't whether Windows has symlinks, since all versions of Windows
supported by PostgreSQL do (AFAIK). The issue is only whether the Perl
installation that runs the tests has symlink support. And that is only
necessary if the test itself wants to create or inspect symlinks. For
example, there are existing tests involving tablespaces that work just fine
on Windows.

Check. Indeed that sounds confusing.

Relatedly, your patch ends up skipping the tests on MSYS2, even though Perl
supports symlinks there out of the box.

Do you think that it would be enough to use what Andrew has mentioned
in [1]/messages/by-id/6c5ffed0-20ee-8878-270f-ab56b7023802@2ndQuadrant.com -- Michael? I don't have a MSYS2 installation, so I am unfortunately not
able to confirm that, but I would just move the check to TestLib.pm
and save it in an extra variable.

[1]: /messages/by-id/6c5ffed0-20ee-8878-270f-ab56b7023802@2ndQuadrant.com -- Michael
--
Michael

#19Michael Paquier
michael@paquier.xyz
In reply to: Michael Paquier (#18)
1 attachment(s)
Re: TAP tests and symlinks on Windows

On Mon, Jun 29, 2020 at 04:56:16PM +0900, Michael Paquier wrote:

On Fri, Jun 26, 2020 at 02:00:37PM +0200, Peter Eisentraut wrote:

We should be more accurate about things like this:

+# The following tests test symlinks. Windows may not have symlinks, so
+# skip there.

The issue isn't whether Windows has symlinks, since all versions of Windows
supported by PostgreSQL do (AFAIK). The issue is only whether the Perl
installation that runs the tests has symlink support. And that is only
necessary if the test itself wants to create or inspect symlinks. For
example, there are existing tests involving tablespaces that work just fine
on Windows.

Check. Indeed that sounds confusing.

Attached is an updated patch, where I have tried to use a better
wording in all the code paths involved.

Relatedly, your patch ends up skipping the tests on MSYS2, even though Perl
supports symlinks there out of the box.

Do you think that it would be enough to use what Andrew has mentioned
in [1]? I don't have a MSYS2 installation, so I am unfortunately not
able to confirm that, but I would just move the check to TestLib.pm
and save it in an extra variable.

Added an extra $is_msys2 to track that in TestLib.pm.  One thing I am
not sure of though: Win32::Symlink fails to work properly with -l, but
is that the case with MSYS2?  If that's able to work, it would be
possible to not skip the following test but I have taken the most
careful approach for now:
+   # This symlink check is not supported on Windows.  Win32::Symlink works
+   # around this situation by using junction points (actually PostgreSQL
+   # approach on the problem), and -l is not able to detect that situation.
+  SKIP:
+   {
+       skip "symlink check not implemented on Windows", 1
+         if ($windows_os)

Thanks,
--
Michael

Attachments:

win32-readlink-tap-v3.patchtext/x-diff; charset=us-asciiDownload
diff --git a/src/bin/pg_basebackup/t/010_pg_basebackup.pl b/src/bin/pg_basebackup/t/010_pg_basebackup.pl
index 208df557b8..7f2df1d09b 100644
--- a/src/bin/pg_basebackup/t/010_pg_basebackup.pl
+++ b/src/bin/pg_basebackup/t/010_pg_basebackup.pl
@@ -211,11 +211,11 @@ $node->command_fails(
 	'pg_basebackup tar with long name fails');
 unlink "$pgdata/$superlongname";
 
-# The following tests test symlinks. Windows doesn't have symlinks, so
-# skip on Windows.
+# The following tests are for symlinks.
 SKIP:
 {
-	skip "symlinks not supported on Windows", 18 if ($windows_os);
+	skip "symlinks not supported with this perl installation", 18
+	  if (!$symlink_support);
 
 	# Move pg_replslot out of $pgdata and create a symlink to it.
 	$node->stop;
@@ -238,11 +238,12 @@ SKIP:
 	# for the tablespace directories, which hopefully won't run afoul of
 	# the 99 character length limit.
 	my $shorter_tempdir = TestLib::tempdir_short . "/tempdir";
+	my $realTsDir       = TestLib::perl2host("$shorter_tempdir/tblspc1");
 	symlink "$tempdir", $shorter_tempdir;
 
 	mkdir "$tempdir/tblspc1";
 	$node->safe_psql('postgres',
-		"CREATE TABLESPACE tblspc1 LOCATION '$shorter_tempdir/tblspc1';");
+		"CREATE TABLESPACE tblspc1 LOCATION '$realTsDir';");
 	$node->safe_psql('postgres',
 		"CREATE TABLE test1 (a int) TABLESPACE tblspc1;");
 	$node->command_ok([ 'pg_basebackup', '-D', "$tempdir/tarbackup2", '-Ft' ],
@@ -292,18 +293,33 @@ SKIP:
 		],
 		'plain format with tablespaces succeeds with tablespace mapping');
 	ok(-d "$tempdir/tbackup/tblspc1", 'tablespace was relocated');
-	opendir(my $dh, "$pgdata/pg_tblspc") or die;
-	ok( (   grep {
-				-l "$tempdir/backup1/pg_tblspc/$_"
-				  and readlink "$tempdir/backup1/pg_tblspc/$_" eq
-				  "$tempdir/tbackup/tblspc1"
-			} readdir($dh)),
-		"tablespace symlink was updated");
-	closedir $dh;
+
+	# This symlink check is not supported on Windows.  Win32::Symlink works
+	# around this situation by using junction points (actually PostgreSQL
+	# approach on the problem), and -l is not able to detect that situation.
+  SKIP:
+	{
+		skip "symlink check not implemented on Windows", 1
+		  if ($windows_os);
+		opendir(my $dh, "$pgdata/pg_tblspc") or die;
+		ok( (   grep {
+					-l "$tempdir/backup1/pg_tblspc/$_"
+					  and readlink "$tempdir/backup1/pg_tblspc/$_" eq
+					  "$tempdir/tbackup/tblspc1"
+				} readdir($dh)),
+			"tablespace symlink was updated");
+		closedir $dh;
+	}
 
 	# Group access should be enabled on all backup files
-	ok(check_mode_recursive("$tempdir/backup1", 0750, 0640),
-		"check backup dir permissions");
+  SKIP:
+	{
+		skip "unix-style permissions not supported on Windows", 1
+		  if ($windows_os);
+
+		ok(check_mode_recursive("$tempdir/backup1", 0750, 0640),
+			"check backup dir permissions");
+	}
 
 	# Unlogged relation forks other than init should not be copied
 	my ($tblspc1UnloggedBackupPath) =
diff --git a/src/bin/pg_dump/t/010_dump_connstr.pl b/src/bin/pg_dump/t/010_dump_connstr.pl
index abdb07c558..1fd8a78abc 100644
--- a/src/bin/pg_dump/t/010_dump_connstr.pl
+++ b/src/bin/pg_dump/t/010_dump_connstr.pl
@@ -5,7 +5,7 @@ use PostgresNode;
 use TestLib;
 use Test::More;
 
-if ($^O eq 'msys' && `uname -or` =~ /^[2-9].*Msys/)
+if ($TestLib::is_msys2)
 {
 	plan skip_all => 'High bit name tests fail on Msys2';
 }
diff --git a/src/bin/pg_rewind/t/004_pg_xlog_symlink.pl b/src/bin/pg_rewind/t/004_pg_xlog_symlink.pl
index 639eeb9c91..400d6d9b7d 100644
--- a/src/bin/pg_rewind/t/004_pg_xlog_symlink.pl
+++ b/src/bin/pg_rewind/t/004_pg_xlog_symlink.pl
@@ -7,9 +7,9 @@ use File::Copy;
 use File::Path qw(rmtree);
 use TestLib;
 use Test::More;
-if ($windows_os)
+if (!$symlink_support)
 {
-	plan skip_all => 'symlinks not supported on Windows';
+	plan skip_all => 'symlinks not supported with this perl installation';
 	exit;
 }
 else
diff --git a/src/test/perl/TestLib.pm b/src/test/perl/TestLib.pm
index d579d5c177..329c656ed1 100644
--- a/src/test/perl/TestLib.pm
+++ b/src/test/perl/TestLib.pm
@@ -22,6 +22,7 @@ TestLib - helper module for writing PostgreSQL's C<prove> tests.
 
   # Miscellanea
   print "on Windows" if $TestLib::windows_os;
+  print "supports symlinks" if $TestLib::symlink_support;
   my $path = TestLib::perl2host($backup_dir);
   ok(check_mode_recursive($stream_dir, 0700, 0600),
     "check stream dir permissions");
@@ -84,10 +85,13 @@ our @EXPORT = qw(
   command_checks_all
 
   $windows_os
+  $symlink_support
+  $is_msys2
   $use_unix_sockets
 );
 
-our ($windows_os, $use_unix_sockets, $tmp_check, $log_path, $test_logfile);
+our ($windows_os, $is_msys2, $symlink_support, $use_unix_sockets, $tmp_check,
+	$log_path, $test_logfile);
 
 BEGIN
 {
@@ -120,6 +124,32 @@ BEGIN
 		Win32API::File->import(qw(createFile OsFHandleOpen CloseHandle));
 	}
 
+	# Check if this environment is MSYS2.
+	$is_msys2 = $^O eq 'msys' && `uname -or` =~ /^[2-9].*Msys/;
+
+	# Check if this installation of perl running the tests has support
+	# for symlinks.  Some installations of perl on Windows may provide
+	# an equivalent implementation based on junction points thanks to
+	# Win32::Symlink.  Non-Windows platforms and MSYS2 are able to
+	# support this case.
+	if ($windows_os && !$is_msys2)
+	{
+		eval { require Win32::Symlink; };
+		if ($@)
+		{
+			$symlink_support = 0;
+		}
+		else
+		{
+			$symlink_support = 1;
+			Win32::Symlink->import(qw(readlink symlink));
+		}
+	}
+	else
+	{
+		$symlink_support = 1;
+	}
+
 	# Specifies whether to use Unix sockets for test setups.  On
 	# Windows we don't use them by default since it's not universally
 	# supported, but it can be overridden if desired.
@@ -133,6 +163,14 @@ BEGIN
 
 =over
 
+=item C<$is_msys2>
+
+Set to true when running under MSYS2.
+
+=item C<$symlink_support>
+
+Set to true when running with an installation of perl that supports symlinks.
+
 =item C<$windows_os>
 
 Set to true when running under Windows, except on Cygwin.
diff --git a/doc/src/sgml/regress.sgml b/doc/src/sgml/regress.sgml
index d98187c970..6fb68db7c1 100644
--- a/doc/src/sgml/regress.sgml
+++ b/doc/src/sgml/regress.sgml
@@ -775,6 +775,9 @@ make check PROVE_TESTS='t/001_test1.pl t/003_test3.pl'
    <para>
     The TAP tests require the Perl module <literal>IPC::Run</literal>.
     This module is available from CPAN or an operating system package.
+    On Windows, <literal>Win32API::File</literal> is additionally
+    required, and <literal>Win32::Symlink</literal> is optional to be
+    able to run tests related to symlinks.
    </para>
 
    <para>
#20Peter Eisentraut
peter.eisentraut@2ndquadrant.com
In reply to: Michael Paquier (#19)
Re: TAP tests and symlinks on Windows

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.

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.

--
Peter Eisentraut http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services

#21Andrew Dunstan
andrew.dunstan@2ndquadrant.com
In reply to: Peter Eisentraut (#20)
Re: TAP tests and symlinks on Windows

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.

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.

I'll see what I can come up with.

cheers

andrew

--
Andrew Dunstan https://www.2ndQuadrant.com
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services

#22Michael Paquier
michael@paquier.xyz
In reply to: Andrew Dunstan (#21)
Re: TAP tests and symlinks on Windows

On Sun, Jul 05, 2020 at 08:18:46AM -0400, Andrew Dunstan wrote:

On 7/3/20 10:11 AM, Peter Eisentraut wrote:

I think all of this is still a bit too fragile it needs further
consideration.

Indeed. I would need a MSYS2 environment to dig into that. This
looks trickier than what I am used to on Windows.

I'll see what I can come up with.

Thanks, Andrew.
--
Michael

#23Andrew Dunstan
andrew.dunstan@2ndquadrant.com
In reply to: Peter Eisentraut (#20)
Re: TAP tests and symlinks on Windows

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

#24Juan José Santamaría Flecha
juanjo.santamaria@gmail.com
In reply to: Andrew Dunstan (#23)
Re: TAP tests and symlinks on Windows

On Wed, Jul 8, 2020 at 3:54 PM Andrew Dunstan <
andrew.dunstan@2ndquadrant.com> wrote:

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.

There is a utility cygpath [1]http://cygwin.net/cygwin-ug-net/cygpath.html meant for the conversion between Unix and
Windows path formats, that might be a meaningful name also.

[1]: http://cygwin.net/cygwin-ug-net/cygpath.html

Regards,

Juan José Santamaría Flecha

#25Andrew Dunstan
andrew.dunstan@2ndquadrant.com
In reply to: Juan José Santamaría Flecha (#24)
Re: TAP tests and symlinks on Windows

On 7/8/20 11:07 AM, Juan José Santamaría Flecha wrote:

On Wed, Jul 8, 2020 at 3:54 PM Andrew Dunstan
<andrew.dunstan@2ndquadrant.com
<mailto:andrew.dunstan@2ndquadrant.com>> wrote:

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.

There is a utility cygpath [1] meant for the conversion between Unix
and Windows path formats, that might be a meaningful name also.

[1] http://cygwin.net/cygwin-ug-net/cygpath.html

Oh, good find. But unfortunately it's not present in my msys1 installations.

So we'll still need to use the 'pwd -W` trick for those.

cheers

andrew

--
Andrew Dunstan https://www.2ndQuadrant.com
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services

#26Alvaro Herrera
alvherre@2ndquadrant.com
In reply to: Andrew Dunstan (#25)
Re: TAP tests and symlinks on Windows

On 2020-Jul-08, Andrew Dunstan wrote:

On 7/8/20 11:07 AM, Juan Jos� Santamar�a Flecha wrote:

There is a utility cygpath [1] meant for the conversion between Unix
and Windows path formats, that might be a meaningful�name also.

[1]�http://cygwin.net/cygwin-ug-net/cygpath.html

Oh, good find. But unfortunately it's not present in my msys1 installations.

So we'll still need to use the 'pwd -W` trick for those.

I think his point is not to use that utility, just to use its name as
the name of the perl routine.

--
�lvaro Herrera https://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services

#27Andrew Dunstan
andrew.dunstan@2ndquadrant.com
In reply to: Alvaro Herrera (#26)
Re: TAP tests and symlinks on Windows

On 7/8/20 12:22 PM, Alvaro Herrera wrote:

On 2020-Jul-08, Andrew Dunstan wrote:

On 7/8/20 11:07 AM, Juan José Santamaría Flecha wrote:

There is a utility cygpath [1] meant for the conversion between Unix
and Windows path formats, that might be a meaningful name also.

[1] http://cygwin.net/cygwin-ug-net/cygpath.html

Oh, good find. But unfortunately it's not present in my msys1 installations.

So we'll still need to use the 'pwd -W` trick for those.

I think his point is not to use that utility, just to use its name as
the name of the perl routine.

That would be wholly misleading, since it's not needed at all when we're
running running under cygwin.

cheers

andrew

--
Andrew Dunstan https://www.2ndQuadrant.com
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services

#28Juan José Santamaría Flecha
juanjo.santamaria@gmail.com
In reply to: Andrew Dunstan (#27)
Re: TAP tests and symlinks on Windows

On Wed, Jul 8, 2020 at 7:18 PM Andrew Dunstan <
andrew.dunstan@2ndquadrant.com> wrote:

On 7/8/20 12:22 PM, Alvaro Herrera wrote:

On 2020-Jul-08, Andrew Dunstan wrote:

On 7/8/20 11:07 AM, Juan José Santamaría Flecha wrote:

There is a utility cygpath [1] meant for the conversion between Unix
and Windows path formats, that might be a meaningful name also.

[1] http://cygwin.net/cygwin-ug-net/cygpath.html

Oh, good find. But unfortunately it's not present in my msys1

installations.

So we'll still need to use the 'pwd -W` trick for those.

I think his point is not to use that utility, just to use its name as
the name of the perl routine.

That would be wholly misleading, since it's not needed at all when we're
running running under cygwin.

MSYS does not include cygpath(), but MSYS2 does. I see why the name could
be confusing outside cygwin, but that is a given and it would point to a
utility that it mimics. Maybe a note for future reference could be enough.

msys_to_windows_path() seems too long, but is hard to misunderstand.

Regards,

Juan José Santamaría Flecha

#29Andrew Dunstan
andrew.dunstan@2ndquadrant.com
In reply to: Andrew Dunstan (#23)
1 attachment(s)
Re: TAP tests and symlinks on Windows

On 7/8/20 9:54 AM, Andrew Dunstan wrote:

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.

After much frustration and gnashing of teeth here's a patch that allows
almost all the TAP tests involving symlinks to work as expected on all
Windows build environments, without requiring an additional Perl module.
I have tested this on a system that is very similar to that running
drongo and fairywren, with both msys2 and� MSVC builds.

I didn't change the name of perl2host - Sufficient unto the day is the
evil thereof. But I did modify it� a) to allow use of cygpath if
available and b) to allow it to succeed if the grandparent directory
exists when cygpath isn't available.

cheers

andrew

--
Andrew Dunstan https://www.2ndQuadrant.com
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services

Attachments:

win-tap-symlink-2.patchtext/x-patch; charset=UTF-8; name=win-tap-symlink-2.patchDownload
diff --git a/doc/src/sgml/regress.sgml b/doc/src/sgml/regress.sgml
index d98187c970..e6e2b2762d 100644
--- a/doc/src/sgml/regress.sgml
+++ b/doc/src/sgml/regress.sgml
@@ -775,6 +775,7 @@ make check PROVE_TESTS='t/001_test1.pl t/003_test3.pl'
    <para>
     The TAP tests require the Perl module <literal>IPC::Run</literal>.
     This module is available from CPAN or an operating system package.
+    On Windows, <literal>Win32API::File</literal> is also required .
    </para>
 
    <para>
diff --git a/src/bin/pg_basebackup/t/010_pg_basebackup.pl b/src/bin/pg_basebackup/t/010_pg_basebackup.pl
index 208df557b8..38b2a35c3b 100644
--- a/src/bin/pg_basebackup/t/010_pg_basebackup.pl
+++ b/src/bin/pg_basebackup/t/010_pg_basebackup.pl
@@ -211,154 +211,168 @@ $node->command_fails(
 	'pg_basebackup tar with long name fails');
 unlink "$pgdata/$superlongname";
 
-# The following tests test symlinks. Windows doesn't have symlinks, so
-# skip on Windows.
+# The following tests are for symlinks.
+
+# Move pg_replslot out of $pgdata and create a symlink to it.
+$node->stop;
+
+# Set umask so test directories and files are created with group permissions
+umask(0027);
+
+# Enable group permissions on PGDATA
+chmod_recursive("$pgdata", 0750, 0640);
+
+rename("$pgdata/pg_replslot", "$tempdir/pg_replslot")
+  or BAIL_OUT "could not move $pgdata/pg_replslot";
+dir_symlink("$tempdir/pg_replslot", "$pgdata/pg_replslot")
+  or BAIL_OUT "could not symlink to $pgdata/pg_replslot";
+
+$node->start;
+
+# Create a temporary directory in the system location and symlink it
+# to our physical temp location.  That way we can use shorter names
+# for the tablespace directories, which hopefully won't run afoul of
+# the 99 character length limit.
+my $shorter_tempdir = TestLib::tempdir_short . "/tempdir";
+dir_symlink "$tempdir", $shorter_tempdir;
+
+mkdir "$tempdir/tblspc1";
+my $realTsDir       = TestLib::perl2host("$shorter_tempdir/tblspc1");
+my $real_tempdir = TestLib::perl2host($tempdir);
+$node->safe_psql('postgres',
+				 "CREATE TABLESPACE tblspc1 LOCATION '$realTsDir';");
+$node->safe_psql('postgres',
+				 "CREATE TABLE test1 (a int) TABLESPACE tblspc1;");
+$node->command_ok([ 'pg_basebackup', '-D', "$real_tempdir/tarbackup2", '-Ft' ],
+				  'tar format with tablespaces');
+ok(-f "$tempdir/tarbackup2/base.tar", 'backup tar was created');
+my @tblspc_tars = glob "$tempdir/tarbackup2/[0-9]*.tar";
+is(scalar(@tblspc_tars), 1, 'one tablespace tar was created');
+rmtree("$tempdir/tarbackup2");
+
+# Create an unlogged table to test that forks other than init are not copied.
+$node->safe_psql('postgres',
+				 'CREATE UNLOGGED TABLE tblspc1_unlogged (id int) TABLESPACE tblspc1;'
+				);
+
+my $tblspc1UnloggedPath = $node->safe_psql('postgres',
+										   q{select pg_relation_filepath('tblspc1_unlogged')});
+
+# Make sure main and init forks exist
+ok( -f "$pgdata/${tblspc1UnloggedPath}_init",
+	'unlogged init fork in tablespace');
+ok(-f "$pgdata/$tblspc1UnloggedPath", 'unlogged main fork in tablespace');
+
+# Create files that look like temporary relations to ensure they are ignored
+# in a tablespace.
+my @tempRelationFiles = qw(t888_888 t888888_888888_vm.1);
+my $tblSpc1Id         = basename(
+	dirname(
+		dirname(
+			$node->safe_psql(
+				'postgres', q{select pg_relation_filepath('test1')}))));
+
+foreach my $filename (@tempRelationFiles)
+{
+	append_to_file(
+		"$shorter_tempdir/tblspc1/$tblSpc1Id/$postgresOid/$filename",
+		'TEMP_RELATION');
+}
+
+$node->command_fails(
+	[ 'pg_basebackup', '-D', "$tempdir/backup1", '-Fp' ],
+	'plain format with tablespaces fails without tablespace mapping');
+
+$node->command_ok(
+	[
+		'pg_basebackup', '-D', "$tempdir/backup1", '-Fp',
+		"-T$realTsDir=$real_tempdir/tbackup/tblspc1"
+	   ],
+	'plain format with tablespaces succeeds with tablespace mapping');
+ok(-d "$tempdir/tbackup/tblspc1", 'tablespace was relocated');
+
+# This symlink check is not supported on Windows as -l
+# doesn't work with junctions
 SKIP:
 {
-	skip "symlinks not supported on Windows", 18 if ($windows_os);
-
-	# Move pg_replslot out of $pgdata and create a symlink to it.
-	$node->stop;
-
-	# Set umask so test directories and files are created with group permissions
-	umask(0027);
-
-	# Enable group permissions on PGDATA
-	chmod_recursive("$pgdata", 0750, 0640);
-
-	rename("$pgdata/pg_replslot", "$tempdir/pg_replslot")
-	  or BAIL_OUT "could not move $pgdata/pg_replslot";
-	symlink("$tempdir/pg_replslot", "$pgdata/pg_replslot")
-	  or BAIL_OUT "could not symlink to $pgdata/pg_replslot";
-
-	$node->start;
-
-	# Create a temporary directory in the system location and symlink it
-	# to our physical temp location.  That way we can use shorter names
-	# for the tablespace directories, which hopefully won't run afoul of
-	# the 99 character length limit.
-	my $shorter_tempdir = TestLib::tempdir_short . "/tempdir";
-	symlink "$tempdir", $shorter_tempdir;
-
-	mkdir "$tempdir/tblspc1";
-	$node->safe_psql('postgres',
-		"CREATE TABLESPACE tblspc1 LOCATION '$shorter_tempdir/tblspc1';");
-	$node->safe_psql('postgres',
-		"CREATE TABLE test1 (a int) TABLESPACE tblspc1;");
-	$node->command_ok([ 'pg_basebackup', '-D', "$tempdir/tarbackup2", '-Ft' ],
-		'tar format with tablespaces');
-	ok(-f "$tempdir/tarbackup2/base.tar", 'backup tar was created');
-	my @tblspc_tars = glob "$tempdir/tarbackup2/[0-9]*.tar";
-	is(scalar(@tblspc_tars), 1, 'one tablespace tar was created');
-	rmtree("$tempdir/tarbackup2");
-
-	# Create an unlogged table to test that forks other than init are not copied.
-	$node->safe_psql('postgres',
-		'CREATE UNLOGGED TABLE tblspc1_unlogged (id int) TABLESPACE tblspc1;'
-	);
-
-	my $tblspc1UnloggedPath = $node->safe_psql('postgres',
-		q{select pg_relation_filepath('tblspc1_unlogged')});
-
-	# Make sure main and init forks exist
-	ok( -f "$pgdata/${tblspc1UnloggedPath}_init",
-		'unlogged init fork in tablespace');
-	ok(-f "$pgdata/$tblspc1UnloggedPath", 'unlogged main fork in tablespace');
-
-	# Create files that look like temporary relations to ensure they are ignored
-	# in a tablespace.
-	my @tempRelationFiles = qw(t888_888 t888888_888888_vm.1);
-	my $tblSpc1Id         = basename(
-		dirname(
-			dirname(
-				$node->safe_psql(
-					'postgres', q{select pg_relation_filepath('test1')}))));
-
-	foreach my $filename (@tempRelationFiles)
-	{
-		append_to_file(
-			"$shorter_tempdir/tblspc1/$tblSpc1Id/$postgresOid/$filename",
-			'TEMP_RELATION');
-	}
-
-	$node->command_fails(
-		[ 'pg_basebackup', '-D', "$tempdir/backup1", '-Fp' ],
-		'plain format with tablespaces fails without tablespace mapping');
-
-	$node->command_ok(
-		[
-			'pg_basebackup', '-D', "$tempdir/backup1", '-Fp',
-			"-T$shorter_tempdir/tblspc1=$tempdir/tbackup/tblspc1"
-		],
-		'plain format with tablespaces succeeds with tablespace mapping');
-	ok(-d "$tempdir/tbackup/tblspc1", 'tablespace was relocated');
+	skip "symlink check not implemented on Windows", 1
+	  if ($windows_os);
 	opendir(my $dh, "$pgdata/pg_tblspc") or die;
 	ok( (   grep {
-				-l "$tempdir/backup1/pg_tblspc/$_"
-				  and readlink "$tempdir/backup1/pg_tblspc/$_" eq
-				  "$tempdir/tbackup/tblspc1"
-			} readdir($dh)),
+		-l "$tempdir/backup1/pg_tblspc/$_"
+		  and readlink "$tempdir/backup1/pg_tblspc/$_" eq
+		  "$tempdir/tbackup/tblspc1"
+	  } readdir($dh)),
 		"tablespace symlink was updated");
 	closedir $dh;
+}
+
+# Group access should be enabled on all backup files
+SKIP:
+{
+	skip "unix-style permissions not supported on Windows", 1
+	  if ($windows_os);
 
-	# Group access should be enabled on all backup files
 	ok(check_mode_recursive("$tempdir/backup1", 0750, 0640),
-		"check backup dir permissions");
+	   "check backup dir permissions");
+}
+
+# Unlogged relation forks other than init should not be copied
+my ($tblspc1UnloggedBackupPath) =
+  $tblspc1UnloggedPath =~ /[^\/]*\/[^\/]*\/[^\/]*$/g;
+
+ok(-f "$tempdir/tbackup/tblspc1/${tblspc1UnloggedBackupPath}_init",
+   'unlogged init fork in tablespace backup');
+ok(!-f "$tempdir/tbackup/tblspc1/$tblspc1UnloggedBackupPath",
+   'unlogged main fork not in tablespace backup');
 
-	# Unlogged relation forks other than init should not be copied
-	my ($tblspc1UnloggedBackupPath) =
-	  $tblspc1UnloggedPath =~ /[^\/]*\/[^\/]*\/[^\/]*$/g;
-
-	ok(-f "$tempdir/tbackup/tblspc1/${tblspc1UnloggedBackupPath}_init",
-		'unlogged init fork in tablespace backup');
-	ok(!-f "$tempdir/tbackup/tblspc1/$tblspc1UnloggedBackupPath",
-		'unlogged main fork not in tablespace backup');
-
-	# Temp relations should not be copied.
-	foreach my $filename (@tempRelationFiles)
-	{
-		ok( !-f "$tempdir/tbackup/tblspc1/$tblSpc1Id/$postgresOid/$filename",
-			"[tblspc1]/$postgresOid/$filename not copied");
-
-		# Also remove temp relation files or tablespace drop will fail.
-		my $filepath =
-		  "$shorter_tempdir/tblspc1/$tblSpc1Id/$postgresOid/$filename";
-
-		unlink($filepath)
-		  or BAIL_OUT("unable to unlink $filepath");
-	}
-
-	ok( -d "$tempdir/backup1/pg_replslot",
-		'pg_replslot symlink copied as directory');
-	rmtree("$tempdir/backup1");
-
-	mkdir "$tempdir/tbl=spc2";
-	$node->safe_psql('postgres', "DROP TABLE test1;");
-	$node->safe_psql('postgres', "DROP TABLE tblspc1_unlogged;");
-	$node->safe_psql('postgres', "DROP TABLESPACE tblspc1;");
-	$node->safe_psql('postgres',
-		"CREATE TABLESPACE tblspc2 LOCATION '$shorter_tempdir/tbl=spc2';");
-	$node->command_ok(
-		[
-			'pg_basebackup', '-D', "$tempdir/backup3", '-Fp',
-			"-T$shorter_tempdir/tbl\\=spc2=$tempdir/tbackup/tbl\\=spc2"
-		],
-		'mapping tablespace with = sign in path');
-	ok(-d "$tempdir/tbackup/tbl=spc2",
-		'tablespace with = sign was relocated');
-	$node->safe_psql('postgres', "DROP TABLESPACE tblspc2;");
-	rmtree("$tempdir/backup3");
-
-	mkdir "$tempdir/$superlongname";
-	$node->safe_psql('postgres',
-		"CREATE TABLESPACE tblspc3 LOCATION '$tempdir/$superlongname';");
-	$node->command_ok(
-		[ 'pg_basebackup', '-D', "$tempdir/tarbackup_l3", '-Ft' ],
-		'pg_basebackup tar with long symlink target');
-	$node->safe_psql('postgres', "DROP TABLESPACE tblspc3;");
-	rmtree("$tempdir/tarbackup_l3");
+# Temp relations should not be copied.
+foreach my $filename (@tempRelationFiles)
+{
+	ok( !-f "$tempdir/tbackup/tblspc1/$tblSpc1Id/$postgresOid/$filename",
+		"[tblspc1]/$postgresOid/$filename not copied");
+
+	# Also remove temp relation files or tablespace drop will fail.
+	my $filepath =
+	  "$shorter_tempdir/tblspc1/$tblSpc1Id/$postgresOid/$filename";
+
+	unlink($filepath)
+	  or BAIL_OUT("unable to unlink $filepath");
 }
 
+ok( -d "$tempdir/backup1/pg_replslot",
+	'pg_replslot symlink copied as directory');
+rmtree("$tempdir/backup1");
+
+mkdir "$tempdir/tbl=spc2";
+$realTsDir = TestLib::perl2host("$shorter_tempdir/tbl=spc2");
+$node->safe_psql('postgres', "DROP TABLE test1;");
+$node->safe_psql('postgres', "DROP TABLE tblspc1_unlogged;");
+$node->safe_psql('postgres', "DROP TABLESPACE tblspc1;");
+$node->safe_psql('postgres',
+				 "CREATE TABLESPACE tblspc2 LOCATION '$realTsDir';");
+$realTsDir =~ s/=/\\=/;
+$node->command_ok(
+	[
+		'pg_basebackup', '-D', "$tempdir/backup3", '-Fp',
+		"-T$realTsDir=$real_tempdir/tbackup/tbl\\=spc2"
+	   ],
+	'mapping tablespace with = sign in path');
+ok(-d "$tempdir/tbackup/tbl=spc2",
+   'tablespace with = sign was relocated');
+$node->safe_psql('postgres', "DROP TABLESPACE tblspc2;");
+rmtree("$tempdir/backup3");
+
+mkdir "$tempdir/$superlongname";
+$realTsDir = TestLib::perl2host("$shorter_tempdir/$superlongname");
+$node->safe_psql('postgres',
+				 "CREATE TABLESPACE tblspc3 LOCATION '$realTsDir';");
+$node->command_ok(
+	[ 'pg_basebackup', '-D', "$tempdir/tarbackup_l3", '-Ft' ],
+	'pg_basebackup tar with long symlink target');
+$node->safe_psql('postgres', "DROP TABLESPACE tblspc3;");
+rmtree("$tempdir/tarbackup_l3");
+
 $node->command_ok([ 'pg_basebackup', '-D', "$tempdir/backupR", '-R' ],
 	'pg_basebackup -R runs');
 ok(-f "$tempdir/backupR/postgresql.auto.conf", 'postgresql.auto.conf exists');
diff --git a/src/bin/pg_dump/t/010_dump_connstr.pl b/src/bin/pg_dump/t/010_dump_connstr.pl
index abdb07c558..1fd8a78abc 100644
--- a/src/bin/pg_dump/t/010_dump_connstr.pl
+++ b/src/bin/pg_dump/t/010_dump_connstr.pl
@@ -5,7 +5,7 @@ use PostgresNode;
 use TestLib;
 use Test::More;
 
-if ($^O eq 'msys' && `uname -or` =~ /^[2-9].*Msys/)
+if ($TestLib::is_msys2)
 {
 	plan skip_all => 'High bit name tests fail on Msys2';
 }
diff --git a/src/bin/pg_rewind/t/004_pg_xlog_symlink.pl b/src/bin/pg_rewind/t/004_pg_xlog_symlink.pl
index 3813543ee1..fff4758508 100644
--- a/src/bin/pg_rewind/t/004_pg_xlog_symlink.pl
+++ b/src/bin/pg_rewind/t/004_pg_xlog_symlink.pl
@@ -6,16 +6,7 @@ use warnings;
 use File::Copy;
 use File::Path qw(rmtree);
 use TestLib;
-use Test::More;
-if ($windows_os)
-{
-	plan skip_all => 'symlinks not supported on Windows';
-	exit;
-}
-else
-{
-	plan tests => 5;
-}
+use Test::More tests => 5;
 
 use FindBin;
 use lib $FindBin::RealBin;
@@ -36,7 +27,7 @@ sub run_test
 	# turn pg_wal into a symlink
 	print("moving $test_primary_datadir/pg_wal to $primary_xlogdir\n");
 	move("$test_primary_datadir/pg_wal", $primary_xlogdir) or die;
-	symlink($primary_xlogdir, "$test_primary_datadir/pg_wal") or die;
+	dir_symlink($primary_xlogdir, "$test_primary_datadir/pg_wal") or die;
 
 	RewindTest::start_primary();
 
diff --git a/src/test/perl/TestLib.pm b/src/test/perl/TestLib.pm
index a7490d2ce7..207e92fdc0 100644
--- a/src/test/perl/TestLib.pm
+++ b/src/test/perl/TestLib.pm
@@ -67,6 +67,7 @@ our @EXPORT = qw(
   check_mode_recursive
   chmod_recursive
   check_pg_config
+  dir_symlink
   system_or_bail
   system_log
   run_log
@@ -84,10 +85,11 @@ our @EXPORT = qw(
   command_checks_all
 
   $windows_os
+  $is_msys2
   $use_unix_sockets
 );
 
-our ($windows_os, $use_unix_sockets, $tmp_check, $log_path, $test_logfile);
+our ($windows_os, $is_msys2, $use_unix_sockets, $tmp_check, $log_path, $test_logfile);
 
 BEGIN
 {
@@ -114,6 +116,9 @@ BEGIN
 
 	# Must be set early
 	$windows_os = $Config{osname} eq 'MSWin32' || $Config{osname} eq 'msys';
+	# Check if this environment is MSYS2.
+	$is_msys2 = $^O eq 'msys' && `uname -or` =~ /^[2-9].*Msys/;
+
 	if ($windows_os)
 	{
 		require Win32API::File;
@@ -137,6 +142,10 @@ BEGIN
 
 Set to true when running under Windows, except on Cygwin.
 
+=item C<$is_msys2>
+
+Set to true when running under MSYS2.
+
 =back
 
 =cut
@@ -263,9 +272,10 @@ sub tempdir_short
 
 =item perl2host()
 
-Translate a Perl file name to a host file name.  Currently, this is a no-op
+Translate a virtual file name to a host file name.  Currently, this is a no-op
 except for the case of Perl=msys and host=mingw32.  The subject need not
-exist, but its parent directory must exist.
+exist, but its parent or grandparent directory must exist unless cygpath is
+available.
 
 =cut
 
@@ -273,6 +283,17 @@ sub perl2host
 {
 	my ($subject) = @_;
 	return $subject unless $Config{osname} eq 'msys';
+	if ($is_msys2)
+	{
+		# get absolute, windows type path
+		my $path = qx{cygpath -a -w "$subject"};
+		if (! $?)
+		{
+			chomp $path;
+			return $path if $path;
+		}
+		# fall through if this didn't work.
+	}
 	my $here = cwd;
 	my $leaf;
 	if (chdir $subject)
@@ -283,7 +304,12 @@ sub perl2host
 	{
 		$leaf = '/' . basename $subject;
 		my $parent = dirname $subject;
-		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\": $!";
+		}
 	}
 
 	# this odd way of calling 'pwd -W' is the only way that seems to work.
@@ -602,6 +628,41 @@ sub check_pg_config
 
 =pod
 
+=item dir_symlink(oldname, newname)
+
+Portably create a symlink for a director. On Windows this creates a junction.
+Elsewhere it just calls perl's builtin symlink.
+
+=cut
+
+sub dir_symlink
+{
+	my $oldname = shift;
+	my $newname = shift;
+	if ($windows_os)
+	{
+		$oldname = perl2host($oldname);
+		$newname = perl2host($newname);
+		$oldname =~ s,/,\\,g;
+		$newname =~ s,/,\\,g;
+		my $cmd = qq{mklink /j "$newname" "$oldname"};
+		if ($Config{osname} eq 'msys')
+		{
+			# need some indirection on msys
+			$cmd = qq{echo '$cmd' | \$COMSPEC /Q};
+		}
+		note("dir_symlink cmd: $cmd");
+		system($cmd);
+	}
+	else
+	{
+		symlink $oldname, $newname;
+	}
+	die "No $newname" unless -e $newname;
+}
+
+=pod
+
 =back
 
 =head1 Test::More-LIKE METHODS
#30Michael Paquier
michael@paquier.xyz
In reply to: Andrew Dunstan (#29)
Re: TAP tests and symlinks on Windows

On Fri, Jul 10, 2020 at 07:58:02AM -0400, Andrew Dunstan wrote:

After much frustration and gnashing of teeth here's a patch that allows
almost all the TAP tests involving symlinks to work as expected on all
Windows build environments, without requiring an additional Perl module.
I have tested this on a system that is very similar to that running
drongo and fairywren, with both msys2 and MSVC builds.

Thanks Andrew for looking at the part with MSYS. The tests pass for
me with MSVC. The trick with mklink is cool. I have not considered
that, and the test code gets simpler.

+       my $cmd = qq{mklink /j "$newname" "$oldname"};
+       if ($Config{osname} eq 'msys')
+       {
+           # need some indirection on msys
+           $cmd = qq{echo '$cmd' | \$COMSPEC /Q};
+       }
+       note("dir_symlink cmd: $cmd");
+       system($cmd);
From the quoting perspective, wouldn't it be simpler to build an array
with all those arguments and call system() with @cmd?
+# Create files that look like temporary relations to ensure they are ignored
+# in a tablespace.
+my @tempRelationFiles = qw(t888_888 t888888_888888_vm.1);
This variable conflicts with a previous declaration, creating a
warning.
+   skip "symlink check not implemented on Windows", 1
+     if ($windows_os);
    opendir(my $dh, "$pgdata/pg_tblspc") or die;
I think that this would be cleaner with a SKIP block.
+Portably create a symlink for a director. On Windows this creates a junction.
+Elsewhere it just calls perl's builtin symlink.
s/director/directory/
s/junction/junction point/

<para>
The TAP tests require the Perl module <literal>IPC::Run</literal>.
This module is available from CPAN or an operating system package.
+ On Windows, <literal>Win32API::File</literal> is also required .
</para>
This part should be backpatched IMO.

Some of the indentation is weird, this needs a cleanup with perltidy.
--
Michael

#31Peter Eisentraut
peter.eisentraut@2ndquadrant.com
In reply to: Andrew Dunstan (#29)
1 attachment(s)
Re: TAP tests and symlinks on Windows

On 2020-07-10 13:58, Andrew Dunstan wrote:

After much frustration and gnashing of teeth here's a patch that allows
almost all the TAP tests involving symlinks to work as expected on all
Windows build environments, without requiring an additional Perl module.
I have tested this on a system that is very similar to that running
drongo and fairywren, with both msys2 and� MSVC builds.

Thanks. This patch works for me in my environment. The code changes
look very clean, so it seems like a good improvement.

Attached is a small fixup patch for some typos and a stray debug message.

A perltidy run might be worthwhile, as Michael already mentioned.

--
Peter Eisentraut http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services

Attachments:

0001-fixup-win-tap-symlink-2.patch.patchtext/plain; charset=UTF-8; name=0001-fixup-win-tap-symlink-2.patch.patch; x-mac-creator=0; x-mac-type=0Download
From 61fc47e7fd82f65ada6a25ddb9ea0458da8e6d5e Mon Sep 17 00:00:00 2001
From: Peter Eisentraut <peter@eisentraut.org>
Date: Tue, 14 Jul 2020 09:11:42 +0200
Subject: [PATCH] fixup! win-tap-symlink-2.patch

---
 doc/src/sgml/regress.sgml | 2 +-
 src/test/perl/TestLib.pm  | 3 +--
 2 files changed, 2 insertions(+), 3 deletions(-)

diff --git a/doc/src/sgml/regress.sgml b/doc/src/sgml/regress.sgml
index e6e2b2762d..4e370aeae4 100644
--- a/doc/src/sgml/regress.sgml
+++ b/doc/src/sgml/regress.sgml
@@ -775,7 +775,7 @@ <title>TAP Tests</title>
    <para>
     The TAP tests require the Perl module <literal>IPC::Run</literal>.
     This module is available from CPAN or an operating system package.
-    On Windows, <literal>Win32API::File</literal> is also required .
+    On Windows, <literal>Win32API::File</literal> is also required.
    </para>
 
    <para>
diff --git a/src/test/perl/TestLib.pm b/src/test/perl/TestLib.pm
index 207e92fdc0..d595c7e3c2 100644
--- a/src/test/perl/TestLib.pm
+++ b/src/test/perl/TestLib.pm
@@ -630,7 +630,7 @@ sub check_pg_config
 
 =item dir_symlink(oldname, newname)
 
-Portably create a symlink for a director. On Windows this creates a junction.
+Portably create a symlink for a directory. On Windows this creates a junction.
 Elsewhere it just calls perl's builtin symlink.
 
 =cut
@@ -651,7 +651,6 @@ sub dir_symlink
 			# need some indirection on msys
 			$cmd = qq{echo '$cmd' | \$COMSPEC /Q};
 		}
-		note("dir_symlink cmd: $cmd");
 		system($cmd);
 	}
 	else
-- 
2.27.0

#32Andrew Dunstan
andrew.dunstan@2ndquadrant.com
In reply to: Michael Paquier (#30)
1 attachment(s)
Re: TAP tests and symlinks on Windows

On 7/14/20 1:31 AM, Michael Paquier wrote:

On Fri, Jul 10, 2020 at 07:58:02AM -0400, Andrew Dunstan wrote:

After much frustration and gnashing of teeth here's a patch that allows
almost all the TAP tests involving symlinks to work as expected on all
Windows build environments, without requiring an additional Perl module.
I have tested this on a system that is very similar to that running
drongo and fairywren, with both msys2 and MSVC builds.

Thanks Andrew for looking at the part with MSYS. The tests pass for
me with MSVC. The trick with mklink is cool. I have not considered
that, and the test code gets simpler.

+       my $cmd = qq{mklink /j "$newname" "$oldname"};
+       if ($Config{osname} eq 'msys')
+       {
+           # need some indirection on msys
+           $cmd = qq{echo '$cmd' | \$COMSPEC /Q};
+       }
+       note("dir_symlink cmd: $cmd");
+       system($cmd);
From the quoting perspective, wouldn't it be simpler to build an array
with all those arguments and call system() with @cmd?

This is the simplest invocation I found to be reliable on msys2 (and it
took me a long time to find). If you have a tested alternative please
let me know.

+# Create files that look like temporary relations to ensure they are ignored
+# in a tablespace.
+my @tempRelationFiles = qw(t888_888 t888888_888888_vm.1);
This variable conflicts with a previous declaration, creating a
warning.
+   skip "symlink check not implemented on Windows", 1
+     if ($windows_os);
opendir(my $dh, "$pgdata/pg_tblspc") or die;
I think that this would be cleaner with a SKIP block.

I don't understand this comment. The skip statement here is in a SKIP
block. In fact skip only works inside SKIP blocks. (perldoc Test::More
for details). Maybe you got confused by the diff format.

+Portably create a symlink for a director. On Windows this creates a junction.
+Elsewhere it just calls perl's builtin symlink.
s/director/directory/
s/junction/junction point/

fixed.

<para>
The TAP tests require the Perl module <literal>IPC::Run</literal>.
This module is available from CPAN or an operating system package.
+ On Windows, <literal>Win32API::File</literal> is also required .
</para>
This part should be backpatched IMO.

I will do this in� a separate backpatched commit.

Some of the indentation is weird, this needs a cleanup with perltidy.

Done.

Revised patch attached.

cheers

andrew

--
Andrew Dunstan https://www.2ndQuadrant.com
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services

Attachments:

win-tap-symlink-3.patchtext/x-patch; charset=UTF-8; name=win-tap-symlink-3.patchDownload
diff --git a/src/bin/pg_basebackup/t/010_pg_basebackup.pl b/src/bin/pg_basebackup/t/010_pg_basebackup.pl
index 208df557b8..f674a7c94e 100644
--- a/src/bin/pg_basebackup/t/010_pg_basebackup.pl
+++ b/src/bin/pg_basebackup/t/010_pg_basebackup.pl
@@ -211,87 +211,93 @@ $node->command_fails(
 	'pg_basebackup tar with long name fails');
 unlink "$pgdata/$superlongname";
 
-# The following tests test symlinks. Windows doesn't have symlinks, so
-# skip on Windows.
+# The following tests are for symlinks.
+
+# Move pg_replslot out of $pgdata and create a symlink to it.
+$node->stop;
+
+# Set umask so test directories and files are created with group permissions
+umask(0027);
+
+# Enable group permissions on PGDATA
+chmod_recursive("$pgdata", 0750, 0640);
+
+rename("$pgdata/pg_replslot", "$tempdir/pg_replslot")
+  or BAIL_OUT "could not move $pgdata/pg_replslot";
+dir_symlink("$tempdir/pg_replslot", "$pgdata/pg_replslot")
+  or BAIL_OUT "could not symlink to $pgdata/pg_replslot";
+
+$node->start;
+
+# Create a temporary directory in the system location and symlink it
+# to our physical temp location.  That way we can use shorter names
+# for the tablespace directories, which hopefully won't run afoul of
+# the 99 character length limit.
+my $shorter_tempdir = TestLib::tempdir_short . "/tempdir";
+dir_symlink "$tempdir", $shorter_tempdir;
+
+mkdir "$tempdir/tblspc1";
+my $realTsDir    = TestLib::perl2host("$shorter_tempdir/tblspc1");
+my $real_tempdir = TestLib::perl2host($tempdir);
+$node->safe_psql('postgres',
+	"CREATE TABLESPACE tblspc1 LOCATION '$realTsDir';");
+$node->safe_psql('postgres',
+	"CREATE TABLE test1 (a int) TABLESPACE tblspc1;");
+$node->command_ok(
+	[ 'pg_basebackup', '-D', "$real_tempdir/tarbackup2", '-Ft' ],
+	'tar format with tablespaces');
+ok(-f "$tempdir/tarbackup2/base.tar", 'backup tar was created');
+my @tblspc_tars = glob "$tempdir/tarbackup2/[0-9]*.tar";
+is(scalar(@tblspc_tars), 1, 'one tablespace tar was created');
+rmtree("$tempdir/tarbackup2");
+
+# Create an unlogged table to test that forks other than init are not copied.
+$node->safe_psql('postgres',
+	'CREATE UNLOGGED TABLE tblspc1_unlogged (id int) TABLESPACE tblspc1;');
+
+my $tblspc1UnloggedPath = $node->safe_psql('postgres',
+	q{select pg_relation_filepath('tblspc1_unlogged')});
+
+# Make sure main and init forks exist
+ok( -f "$pgdata/${tblspc1UnloggedPath}_init",
+	'unlogged init fork in tablespace');
+ok(-f "$pgdata/$tblspc1UnloggedPath", 'unlogged main fork in tablespace');
+
+# Create files that look like temporary relations to ensure they are ignored
+# in a tablespace.
+@tempRelationFiles = qw(t888_888 t888888_888888_vm.1);
+my $tblSpc1Id = basename(
+	dirname(
+		dirname(
+			$node->safe_psql(
+				'postgres', q{select pg_relation_filepath('test1')}))));
+
+foreach my $filename (@tempRelationFiles)
+{
+	append_to_file(
+		"$shorter_tempdir/tblspc1/$tblSpc1Id/$postgresOid/$filename",
+		'TEMP_RELATION');
+}
+
+$node->command_fails(
+	[ 'pg_basebackup', '-D', "$tempdir/backup1", '-Fp' ],
+	'plain format with tablespaces fails without tablespace mapping');
+
+$node->command_ok(
+	[
+		'pg_basebackup',    '-D',
+		"$tempdir/backup1", '-Fp',
+		"-T$realTsDir=$real_tempdir/tbackup/tblspc1"
+	],
+	'plain format with tablespaces succeeds with tablespace mapping');
+ok(-d "$tempdir/tbackup/tblspc1", 'tablespace was relocated');
+
+# This symlink check is not supported on Windows as -l
+# doesn't work with junctions
 SKIP:
 {
-	skip "symlinks not supported on Windows", 18 if ($windows_os);
-
-	# Move pg_replslot out of $pgdata and create a symlink to it.
-	$node->stop;
-
-	# Set umask so test directories and files are created with group permissions
-	umask(0027);
-
-	# Enable group permissions on PGDATA
-	chmod_recursive("$pgdata", 0750, 0640);
-
-	rename("$pgdata/pg_replslot", "$tempdir/pg_replslot")
-	  or BAIL_OUT "could not move $pgdata/pg_replslot";
-	symlink("$tempdir/pg_replslot", "$pgdata/pg_replslot")
-	  or BAIL_OUT "could not symlink to $pgdata/pg_replslot";
-
-	$node->start;
-
-	# Create a temporary directory in the system location and symlink it
-	# to our physical temp location.  That way we can use shorter names
-	# for the tablespace directories, which hopefully won't run afoul of
-	# the 99 character length limit.
-	my $shorter_tempdir = TestLib::tempdir_short . "/tempdir";
-	symlink "$tempdir", $shorter_tempdir;
-
-	mkdir "$tempdir/tblspc1";
-	$node->safe_psql('postgres',
-		"CREATE TABLESPACE tblspc1 LOCATION '$shorter_tempdir/tblspc1';");
-	$node->safe_psql('postgres',
-		"CREATE TABLE test1 (a int) TABLESPACE tblspc1;");
-	$node->command_ok([ 'pg_basebackup', '-D', "$tempdir/tarbackup2", '-Ft' ],
-		'tar format with tablespaces');
-	ok(-f "$tempdir/tarbackup2/base.tar", 'backup tar was created');
-	my @tblspc_tars = glob "$tempdir/tarbackup2/[0-9]*.tar";
-	is(scalar(@tblspc_tars), 1, 'one tablespace tar was created');
-	rmtree("$tempdir/tarbackup2");
-
-	# Create an unlogged table to test that forks other than init are not copied.
-	$node->safe_psql('postgres',
-		'CREATE UNLOGGED TABLE tblspc1_unlogged (id int) TABLESPACE tblspc1;'
-	);
-
-	my $tblspc1UnloggedPath = $node->safe_psql('postgres',
-		q{select pg_relation_filepath('tblspc1_unlogged')});
-
-	# Make sure main and init forks exist
-	ok( -f "$pgdata/${tblspc1UnloggedPath}_init",
-		'unlogged init fork in tablespace');
-	ok(-f "$pgdata/$tblspc1UnloggedPath", 'unlogged main fork in tablespace');
-
-	# Create files that look like temporary relations to ensure they are ignored
-	# in a tablespace.
-	my @tempRelationFiles = qw(t888_888 t888888_888888_vm.1);
-	my $tblSpc1Id         = basename(
-		dirname(
-			dirname(
-				$node->safe_psql(
-					'postgres', q{select pg_relation_filepath('test1')}))));
-
-	foreach my $filename (@tempRelationFiles)
-	{
-		append_to_file(
-			"$shorter_tempdir/tblspc1/$tblSpc1Id/$postgresOid/$filename",
-			'TEMP_RELATION');
-	}
-
-	$node->command_fails(
-		[ 'pg_basebackup', '-D', "$tempdir/backup1", '-Fp' ],
-		'plain format with tablespaces fails without tablespace mapping');
-
-	$node->command_ok(
-		[
-			'pg_basebackup', '-D', "$tempdir/backup1", '-Fp',
-			"-T$shorter_tempdir/tblspc1=$tempdir/tbackup/tblspc1"
-		],
-		'plain format with tablespaces succeeds with tablespace mapping');
-	ok(-d "$tempdir/tbackup/tblspc1", 'tablespace was relocated');
+	skip "symlink check not implemented on Windows", 1
+	  if ($windows_os);
 	opendir(my $dh, "$pgdata/pg_tblspc") or die;
 	ok( (   grep {
 				-l "$tempdir/backup1/pg_tblspc/$_"
@@ -300,65 +306,73 @@ SKIP:
 			} readdir($dh)),
 		"tablespace symlink was updated");
 	closedir $dh;
+}
+
+# Group access should be enabled on all backup files
+SKIP:
+{
+	skip "unix-style permissions not supported on Windows", 1
+	  if ($windows_os);
 
-	# Group access should be enabled on all backup files
 	ok(check_mode_recursive("$tempdir/backup1", 0750, 0640),
 		"check backup dir permissions");
+}
+
+# Unlogged relation forks other than init should not be copied
+my ($tblspc1UnloggedBackupPath) =
+  $tblspc1UnloggedPath =~ /[^\/]*\/[^\/]*\/[^\/]*$/g;
+
+ok(-f "$tempdir/tbackup/tblspc1/${tblspc1UnloggedBackupPath}_init",
+	'unlogged init fork in tablespace backup');
+ok(!-f "$tempdir/tbackup/tblspc1/$tblspc1UnloggedBackupPath",
+	'unlogged main fork not in tablespace backup');
 
-	# Unlogged relation forks other than init should not be copied
-	my ($tblspc1UnloggedBackupPath) =
-	  $tblspc1UnloggedPath =~ /[^\/]*\/[^\/]*\/[^\/]*$/g;
-
-	ok(-f "$tempdir/tbackup/tblspc1/${tblspc1UnloggedBackupPath}_init",
-		'unlogged init fork in tablespace backup');
-	ok(!-f "$tempdir/tbackup/tblspc1/$tblspc1UnloggedBackupPath",
-		'unlogged main fork not in tablespace backup');
-
-	# Temp relations should not be copied.
-	foreach my $filename (@tempRelationFiles)
-	{
-		ok( !-f "$tempdir/tbackup/tblspc1/$tblSpc1Id/$postgresOid/$filename",
-			"[tblspc1]/$postgresOid/$filename not copied");
-
-		# Also remove temp relation files or tablespace drop will fail.
-		my $filepath =
-		  "$shorter_tempdir/tblspc1/$tblSpc1Id/$postgresOid/$filename";
-
-		unlink($filepath)
-		  or BAIL_OUT("unable to unlink $filepath");
-	}
-
-	ok( -d "$tempdir/backup1/pg_replslot",
-		'pg_replslot symlink copied as directory');
-	rmtree("$tempdir/backup1");
-
-	mkdir "$tempdir/tbl=spc2";
-	$node->safe_psql('postgres', "DROP TABLE test1;");
-	$node->safe_psql('postgres', "DROP TABLE tblspc1_unlogged;");
-	$node->safe_psql('postgres', "DROP TABLESPACE tblspc1;");
-	$node->safe_psql('postgres',
-		"CREATE TABLESPACE tblspc2 LOCATION '$shorter_tempdir/tbl=spc2';");
-	$node->command_ok(
-		[
-			'pg_basebackup', '-D', "$tempdir/backup3", '-Fp',
-			"-T$shorter_tempdir/tbl\\=spc2=$tempdir/tbackup/tbl\\=spc2"
-		],
-		'mapping tablespace with = sign in path');
-	ok(-d "$tempdir/tbackup/tbl=spc2",
-		'tablespace with = sign was relocated');
-	$node->safe_psql('postgres', "DROP TABLESPACE tblspc2;");
-	rmtree("$tempdir/backup3");
-
-	mkdir "$tempdir/$superlongname";
-	$node->safe_psql('postgres',
-		"CREATE TABLESPACE tblspc3 LOCATION '$tempdir/$superlongname';");
-	$node->command_ok(
-		[ 'pg_basebackup', '-D', "$tempdir/tarbackup_l3", '-Ft' ],
-		'pg_basebackup tar with long symlink target');
-	$node->safe_psql('postgres', "DROP TABLESPACE tblspc3;");
-	rmtree("$tempdir/tarbackup_l3");
+# Temp relations should not be copied.
+foreach my $filename (@tempRelationFiles)
+{
+	ok(!-f "$tempdir/tbackup/tblspc1/$tblSpc1Id/$postgresOid/$filename",
+		"[tblspc1]/$postgresOid/$filename not copied");
+
+	# Also remove temp relation files or tablespace drop will fail.
+	my $filepath =
+	  "$shorter_tempdir/tblspc1/$tblSpc1Id/$postgresOid/$filename";
+
+	unlink($filepath)
+	  or BAIL_OUT("unable to unlink $filepath");
 }
 
+ok( -d "$tempdir/backup1/pg_replslot",
+	'pg_replslot symlink copied as directory');
+rmtree("$tempdir/backup1");
+
+mkdir "$tempdir/tbl=spc2";
+$realTsDir = TestLib::perl2host("$shorter_tempdir/tbl=spc2");
+$node->safe_psql('postgres', "DROP TABLE test1;");
+$node->safe_psql('postgres', "DROP TABLE tblspc1_unlogged;");
+$node->safe_psql('postgres', "DROP TABLESPACE tblspc1;");
+$node->safe_psql('postgres',
+	"CREATE TABLESPACE tblspc2 LOCATION '$realTsDir';");
+$realTsDir =~ s/=/\\=/;
+$node->command_ok(
+	[
+		'pg_basebackup',    '-D',
+		"$tempdir/backup3", '-Fp',
+		"-T$realTsDir=$real_tempdir/tbackup/tbl\\=spc2"
+	],
+	'mapping tablespace with = sign in path');
+ok(-d "$tempdir/tbackup/tbl=spc2", 'tablespace with = sign was relocated');
+$node->safe_psql('postgres', "DROP TABLESPACE tblspc2;");
+rmtree("$tempdir/backup3");
+
+mkdir "$tempdir/$superlongname";
+$realTsDir = TestLib::perl2host("$shorter_tempdir/$superlongname");
+$node->safe_psql('postgres',
+	"CREATE TABLESPACE tblspc3 LOCATION '$realTsDir';");
+$node->command_ok([ 'pg_basebackup', '-D', "$tempdir/tarbackup_l3", '-Ft' ],
+	'pg_basebackup tar with long symlink target');
+$node->safe_psql('postgres', "DROP TABLESPACE tblspc3;");
+rmtree("$tempdir/tarbackup_l3");
+
 $node->command_ok([ 'pg_basebackup', '-D', "$tempdir/backupR", '-R' ],
 	'pg_basebackup -R runs');
 ok(-f "$tempdir/backupR/postgresql.auto.conf", 'postgresql.auto.conf exists');
@@ -496,7 +510,7 @@ my $file_corrupt2 = $node->safe_psql('postgres',
 
 # set page header and block sizes
 my $pageheader_size = 24;
-my $block_size = $node->safe_psql('postgres', 'SHOW block_size;');
+my $block_size      = $node->safe_psql('postgres', 'SHOW block_size;');
 
 # induce corruption
 system_or_bail 'pg_ctl', '-D', $pgdata, 'stop';
diff --git a/src/bin/pg_dump/t/010_dump_connstr.pl b/src/bin/pg_dump/t/010_dump_connstr.pl
index abdb07c558..5497e46056 100644
--- a/src/bin/pg_dump/t/010_dump_connstr.pl
+++ b/src/bin/pg_dump/t/010_dump_connstr.pl
@@ -5,7 +5,7 @@ use PostgresNode;
 use TestLib;
 use Test::More;
 
-if ($^O eq 'msys' && `uname -or` =~ /^[2-9].*Msys/)
+if ($TestLib::is_msys2)
 {
 	plan skip_all => 'High bit name tests fail on Msys2';
 }
@@ -27,7 +27,7 @@ $ENV{PGCLIENTENCODING} = 'LATIN1';
 # The odds of finding something interesting by testing all ASCII letters
 # seem too small to justify the cycles of testing a fifth name.
 my $dbname1 =
-    'regression'
+	'regression'
   . generate_ascii_string(1,  9)
   . generate_ascii_string(11, 12)
   . generate_ascii_string(14, 33)
diff --git a/src/bin/pg_rewind/t/004_pg_xlog_symlink.pl b/src/bin/pg_rewind/t/004_pg_xlog_symlink.pl
index 3813543ee1..fff4758508 100644
--- a/src/bin/pg_rewind/t/004_pg_xlog_symlink.pl
+++ b/src/bin/pg_rewind/t/004_pg_xlog_symlink.pl
@@ -6,16 +6,7 @@ use warnings;
 use File::Copy;
 use File::Path qw(rmtree);
 use TestLib;
-use Test::More;
-if ($windows_os)
-{
-	plan skip_all => 'symlinks not supported on Windows';
-	exit;
-}
-else
-{
-	plan tests => 5;
-}
+use Test::More tests => 5;
 
 use FindBin;
 use lib $FindBin::RealBin;
@@ -36,7 +27,7 @@ sub run_test
 	# turn pg_wal into a symlink
 	print("moving $test_primary_datadir/pg_wal to $primary_xlogdir\n");
 	move("$test_primary_datadir/pg_wal", $primary_xlogdir) or die;
-	symlink($primary_xlogdir, "$test_primary_datadir/pg_wal") or die;
+	dir_symlink($primary_xlogdir, "$test_primary_datadir/pg_wal") or die;
 
 	RewindTest::start_primary();
 
diff --git a/src/test/perl/TestLib.pm b/src/test/perl/TestLib.pm
index a7490d2ce7..cbe87f8684 100644
--- a/src/test/perl/TestLib.pm
+++ b/src/test/perl/TestLib.pm
@@ -67,6 +67,7 @@ our @EXPORT = qw(
   check_mode_recursive
   chmod_recursive
   check_pg_config
+  dir_symlink
   system_or_bail
   system_log
   run_log
@@ -84,10 +85,12 @@ our @EXPORT = qw(
   command_checks_all
 
   $windows_os
+  $is_msys2
   $use_unix_sockets
 );
 
-our ($windows_os, $use_unix_sockets, $tmp_check, $log_path, $test_logfile);
+our ($windows_os, $is_msys2, $use_unix_sockets, $tmp_check, $log_path,
+	$test_logfile);
 
 BEGIN
 {
@@ -114,6 +117,9 @@ BEGIN
 
 	# Must be set early
 	$windows_os = $Config{osname} eq 'MSWin32' || $Config{osname} eq 'msys';
+	# Check if this environment is MSYS2.
+	$is_msys2 = $^O eq 'msys' && `uname -or` =~ /^[2-9].*Msys/;
+
 	if ($windows_os)
 	{
 		require Win32API::File;
@@ -137,6 +143,10 @@ BEGIN
 
 Set to true when running under Windows, except on Cygwin.
 
+=item C<$is_msys2>
+
+Set to true when running under MSYS2.
+
 =back
 
 =cut
@@ -152,7 +162,7 @@ INIT
 	# TESTDIR environment variable, which is normally set by the invoking
 	# Makefile.
 	$tmp_check = $ENV{TESTDIR} ? "$ENV{TESTDIR}/tmp_check" : "tmp_check";
-	$log_path = "$tmp_check/log";
+	$log_path  = "$tmp_check/log";
 
 	mkdir $tmp_check;
 	mkdir $log_path;
@@ -263,9 +273,10 @@ sub tempdir_short
 
 =item perl2host()
 
-Translate a Perl file name to a host file name.  Currently, this is a no-op
+Translate a virtual file name to a host file name.  Currently, this is a no-op
 except for the case of Perl=msys and host=mingw32.  The subject need not
-exist, but its parent directory must exist.
+exist, but its parent or grandparent directory must exist unless cygpath is
+available.
 
 =cut
 
@@ -273,6 +284,17 @@ sub perl2host
 {
 	my ($subject) = @_;
 	return $subject unless $Config{osname} eq 'msys';
+	if ($is_msys2)
+	{
+		# get absolute, windows type path
+		my $path = qx{cygpath -a -w "$subject"};
+		if (!$?)
+		{
+			chomp $path;
+			return $path if $path;
+		}
+		# fall through if this didn't work.
+	}
 	my $here = cwd;
 	my $leaf;
 	if (chdir $subject)
@@ -283,7 +305,12 @@ sub perl2host
 	{
 		$leaf = '/' . basename $subject;
 		my $parent = dirname $subject;
-		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\": $!";
+		}
 	}
 
 	# this odd way of calling 'pwd -W' is the only way that seems to work.
@@ -602,6 +629,40 @@ sub check_pg_config
 
 =pod
 
+=item dir_symlink(oldname, newname)
+
+Portably create a symlink for a directory. On Windows this creates a junction
+point. Elsewhere it just calls perl's builtin symlink.
+
+=cut
+
+sub dir_symlink
+{
+	my $oldname = shift;
+	my $newname = shift;
+	if ($windows_os)
+	{
+		$oldname = perl2host($oldname);
+		$newname = perl2host($newname);
+		$oldname =~ s,/,\\,g;
+		$newname =~ s,/,\\,g;
+		my $cmd = qq{mklink /j "$newname" "$oldname"};
+		if ($Config{osname} eq 'msys')
+		{
+			# need some indirection on msys
+			$cmd = qq{echo '$cmd' | \$COMSPEC /Q};
+		}
+		system($cmd);
+	}
+	else
+	{
+		symlink $oldname, $newname;
+	}
+	die "No $newname" unless -e $newname;
+}
+
+=pod
+
 =back
 
 =head1 Test::More-LIKE METHODS
@@ -664,7 +725,7 @@ sub command_exit_is
 	# long as the process was not terminated by an exception. To work around
 	# that, use $h->full_results on Windows instead.
 	my $result =
-	    ($Config{osname} eq "MSWin32")
+		($Config{osname} eq "MSWin32")
 	  ? ($h->full_results)[0]
 	  : $h->result(0);
 	is($result, $expected, $test_name);
#33Michael Paquier
michael@paquier.xyz
In reply to: Andrew Dunstan (#32)
Re: TAP tests and symlinks on Windows

On Wed, Jul 15, 2020 at 11:04:28AM -0400, Andrew Dunstan wrote:

This is the simplest invocation I found to be reliable on msys2 (and it
took me a long time to find). If you have a tested alternative please
let me know.

Having a working MSYS environment is still on my TODO list :)

I don't understand this comment. The skip statement here is in a SKIP
block. In fact skip only works inside SKIP blocks. (perldoc Test::More
for details). Maybe you got confused by the diff format.

Indeed, I got trapped by the diff here. Thanks.

The patch looks good to me.
--
Michael

#34Michael Paquier
michael@paquier.xyz
In reply to: Michael Paquier (#33)
Re: TAP tests and symlinks on Windows

On Thu, Jul 16, 2020 at 07:21:27PM +0900, Michael Paquier wrote:

The patch looks good to me.

For the sake of the archives, this has been applied as d66b23b0 and
the buildfarm is green. I have also changed the related CF entry to
reflect what has been done, with Andrew as author, etc:
https://commitfest.postgresql.org/28/2612/
--
Michael