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
signature.asc
Description: PGP signature