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
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

Attachment: signature.asc
Description: PGP signature

Reply via email to